Skip to content

Commit 2f67924

Browse files
authored
Fixes #26824: filter column bulk operations metadata status on the aggregate row (#32905)
* Fixes #26824: filter column-grid metadata status on the aggregate row, not per document The "Has / Missing Metadata" filter in Column Bulk Operations did not hide rows of other statuses, and page counts drifted per page. metadataStatus (MISSING/INCOMPLETE/COMPLETE) was pushed down as a per-document search query, while INCONSISTENT/hasConflicts/hasMissingMetadata were applied after the aggregator had already paginated and computed totals. But a row's status is an aggregate over all of a column's occurrences (INCONSISTENT when they disagree), so a per-document filter re-grouped into rows whose status differed from the request, and the post-pagination filter shrank the page while the total stayed unfiltered. Move every row-level filter (metadataStatus, hasConflicts, hasMissingMetadata) onto the fully-grouped items, before pagination, with totals derived from the filtered set. Shared pure helpers (hasRowLevelFilter, matchesRowFilters, paginateFilteredItems) live on the ColumnAggregator interface; both aggregators route through a materialized name-enumeration path when such a filter is active, and the untouched composite/pattern paths still serve unfiltered browsing. Removing the per-document status query also removes the wildcard/exists query on flat-object columns.description/columns.tags that crashed ES/OS with search_phase_execution_exception and had ColumnGridResourceIT disabled; the IT is re-enabled and its status assertions strengthened to check every returned row carries the requested status, plus a status+pagination consistency test. * perf(search): back column-grid status filter with a field-filtered _source scan Replace the row-filter path's names-agg + per-name top_hits fan-out (~1+N queries/page) with a single _source scan per field-path group — the same mechanism the tag/glossary filter already uses — restricting _source to the column tree and entity-identity fields. This cuts queries per page from ~1+N to ~1 per field-path group, reuses the already-verified in-memory pagination, and reads every occurrence of a column instead of a 100-doc top_hits sample, so the aggregate status can no longer be misclassified (e.g. COMPLETE vs INCONSISTENT) by under-sampling. Restricting _source to columns + identity fields keeps the per-entity payload small, dropping the heavy entity-level derived fields (columnNames/columnNamesFuzzy). extractMatchingColumnsFromHit gains an includeAllColumns flag (the tag path passes false; the status scan passes true); the now-unused name-enumeration constants are removed. * fix(search): honor columnNamePattern per column in the status-filter scan The _source-scan row-filter path returned every column of each matching entity (includeAllColumns=true). Combined with a columnNamePattern the entity-level name wildcard only decides which entities are scanned — flat-object mapping can't isolate the matching column — so non-matching columns leaked into the grid and inflated totalUniqueColumns. Drop columns whose name doesn't contain the pattern after the scan (mirrors the tag path), in both ES and OS aggregators. Also correct the row-filter Javadocs: OS still described the old name-enumeration path, and both overstated "reads all occurrences" — it reads every occurrence of each scanned entity, up to the 10K-entity scan cap. Adds an IT for the columnNamePattern + metadataStatus combination. * refactor(search): extract shared applyColumnNamePattern helper The per-column name-pattern drop-loop was duplicated verbatim in the ES and OS row-filter scans. Move it to a static ColumnAggregator.applyColumnNamePattern helper (alongside the other shared row-filter helpers) so the two backends can't drift in pattern semantics.
1 parent 6d64d2e commit 2f67924

7 files changed

Lines changed: 566 additions & 168 deletions

File tree

openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/ColumnGridResourceIT.java

Lines changed: 134 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import java.time.Duration;
1414
import java.util.List;
1515
import org.junit.jupiter.api.BeforeAll;
16-
import org.junit.jupiter.api.Disabled;
1716
import org.junit.jupiter.api.Test;
1817
import org.junit.jupiter.api.extension.ExtendWith;
1918
import org.junit.jupiter.api.parallel.Execution;
@@ -53,17 +52,10 @@
5352
import org.openmetadata.sdk.fluent.Tables;
5453
import org.openmetadata.sdk.network.HttpMethod;
5554

56-
// TEMPORARILY DISABLED — the metadataStatus aggregation on this endpoint reproducibly fails
57-
// with [search_phase_execution_exception] all shards failed on both postgres+ES+redis (single
58-
// failure on test_getColumnGrid_withMetadataStatusIncomplete) AND postgres+OpenSearch (the same
59-
// query crashes the OS container, then 15 follow-up tests in the class fail with Connection
60-
// refused). Same behavior on PR #28100 with and without the cache changes, so it is a
61-
// pre-existing aggregator bug, not a cache regression. The ES Java client swallows the
62-
// underlying `caused_by`, so root-causing the actual ES-side error requires response-body
63-
// logging that is not wired up yet. Re-enable once the underlying aggregator/index-mapping
64-
// issue is fixed in a follow-up. See PR #28100 history and CI run 25940411417 for context.
65-
@Disabled(
66-
"ColumnGrid metadataStatus aggregation crashes ES/OS — pre-existing flake, follow-up needed")
55+
// Re-enabled with #26824: the metadataStatus crash came from the per-document filter query
56+
// (wildcard/exists on flat-object columns.description/columns.tags), which ES 7.17 and OpenSearch
57+
// rejected with `search_phase_execution_exception ... all shards failed`. That push-down is gone —
58+
// status is now filtered on the aggregate grouped item — so the crashing query no longer runs.
6759
@Execution(ExecutionMode.CONCURRENT)
6860
@ExtendWith(TestNamespaceExtension.class)
6961
public class ColumnGridResourceIT {
@@ -314,6 +306,11 @@ void test_getColumnGrid_withMetadataStatusMissing(TestNamespace ns) throws Excep
314306

315307
assertNotNull(response);
316308
assertNotNull(response.getColumns());
309+
assertAllRowsHaveStatus(response, MetadataStatus.MISSING);
310+
assertEquals(
311+
response.getColumns().size(),
312+
response.getTotalUniqueColumns(),
313+
"totalUniqueColumns must reflect the filtered set, not the unfiltered total");
317314
}
318315

319316
@Test
@@ -328,6 +325,9 @@ void test_getColumnGrid_withMetadataStatusComplete(TestNamespace ns) throws Exce
328325

329326
assertNotNull(response);
330327
assertNotNull(response.getColumns());
328+
assertFalse(response.getColumns().isEmpty(), "the COMPLETE column should be returned");
329+
// The reported bug (#26824): COMPLETE must not surface MISSING/INCOMPLETE/INCONSISTENT rows.
330+
assertAllRowsHaveStatus(response, MetadataStatus.COMPLETE);
331331
}
332332

333333
@Test
@@ -342,6 +342,8 @@ void test_getColumnGrid_withMetadataStatusIncomplete(TestNamespace ns) throws Ex
342342

343343
assertNotNull(response);
344344
assertNotNull(response.getColumns());
345+
assertFalse(response.getColumns().isEmpty(), "the INCOMPLETE column should be returned");
346+
assertAllRowsHaveStatus(response, MetadataStatus.INCOMPLETE);
345347
}
346348

347349
@Test
@@ -422,6 +424,125 @@ void test_getColumnGrid_withMetadataStatusInconsistent(TestNamespace ns) throws
422424

423425
assertNotNull(response);
424426
assertNotNull(response.getColumns());
427+
assertFalse(response.getColumns().isEmpty(), "the INCONSISTENT column should be returned");
428+
assertAllRowsHaveStatus(response, MetadataStatus.INCONSISTENT);
429+
assertTrue(
430+
response.getColumns().stream().allMatch(ColumnGridItem::getHasVariations),
431+
"INCONSISTENT rows have metadata variations across occurrences");
432+
}
433+
434+
@Test
435+
void test_getColumnGrid_metadataStatusPaginationCountsAreConsistent(TestNamespace ns)
436+
throws Exception {
437+
OpenMetadataClient client = SdkClients.adminClient();
438+
DatabaseService service = DatabaseServiceTestFactory.createPostgres(ns);
439+
DatabaseSchema schema = DatabaseSchemaTestFactory.createSimple(ns, service);
440+
441+
// Three COMPLETE columns and one MISSING column in the same service.
442+
for (int i = 0; i < 3; i++) {
443+
Column complete =
444+
Columns.build(ns.prefix("paged_complete_" + i))
445+
.withType(ColumnDataType.BIGINT)
446+
.withDescription("has description")
447+
.withTags(List.of(new TagLabel().withTagFQN("PII.Sensitive")))
448+
.create();
449+
Tables.create()
450+
.name(ns.prefix("paged_table_" + i))
451+
.inSchema(schema.getFullyQualifiedName())
452+
.withColumns(List.of(complete))
453+
.execute();
454+
}
455+
Column missing =
456+
Columns.build(ns.prefix("paged_missing")).withType(ColumnDataType.BIGINT).create();
457+
Tables.create()
458+
.name(ns.prefix("paged_table_missing"))
459+
.inSchema(schema.getFullyQualifiedName())
460+
.withColumns(List.of(missing))
461+
.execute();
462+
463+
waitForSearchIndexRefresh(ns);
464+
465+
ColumnGridResponse page1 =
466+
getColumnGrid(
467+
client,
468+
"size=2&entityTypes=table&metadataStatus=COMPLETE&serviceName=" + service.getName());
469+
470+
// totalUniqueColumns must count only the 3 COMPLETE columns (not 4), and the page must respect
471+
// the requested size — the pagination half of #26824.
472+
assertEquals(3, page1.getTotalUniqueColumns());
473+
assertEquals(2, page1.getColumns().size());
474+
assertAllRowsHaveStatus(page1, MetadataStatus.COMPLETE);
475+
assertNotNull(page1.getCursor(), "a second page of COMPLETE columns remains");
476+
477+
ColumnGridResponse page2 =
478+
getColumnGrid(
479+
client,
480+
"size=2&entityTypes=table&metadataStatus=COMPLETE&serviceName="
481+
+ service.getName()
482+
+ "&cursor="
483+
+ URLEncoder.encode(page1.getCursor(), StandardCharsets.UTF_8));
484+
485+
assertEquals(3, page2.getTotalUniqueColumns());
486+
assertEquals(1, page2.getColumns().size(), "the last page holds the remaining COMPLETE column");
487+
assertAllRowsHaveStatus(page2, MetadataStatus.COMPLETE);
488+
}
489+
490+
@Test
491+
void test_getColumnGrid_metadataStatusWithColumnNamePattern(TestNamespace ns) throws Exception {
492+
OpenMetadataClient client = SdkClients.adminClient();
493+
DatabaseService service = DatabaseServiceTestFactory.createPostgres(ns);
494+
DatabaseSchema schema = DatabaseSchemaTestFactory.createSimple(ns, service);
495+
496+
// Two COMPLETE columns; only one name contains "alpha".
497+
String matchName = ns.prefix("alpha_amount");
498+
String otherName = ns.prefix("zzz_other");
499+
for (String colName : List.of(matchName, otherName)) {
500+
Column col =
501+
Columns.build(colName)
502+
.withType(ColumnDataType.BIGINT)
503+
.withDescription("has description")
504+
.withTags(List.of(new TagLabel().withTagFQN("PII.Sensitive")))
505+
.create();
506+
Tables.create()
507+
.name(ns.prefix("pat_" + colName))
508+
.inSchema(schema.getFullyQualifiedName())
509+
.withColumns(List.of(col))
510+
.execute();
511+
}
512+
waitForSearchIndexRefresh(ns);
513+
514+
ColumnGridResponse response =
515+
getColumnGrid(
516+
client,
517+
"entityTypes=table&metadataStatus=COMPLETE&columnNamePattern=alpha&serviceName="
518+
+ service.getName());
519+
520+
// Combining columnNamePattern with a status filter must honor the pattern per column:
521+
// the non-matching "zzz_other" column must not leak in (regression for the _source-scan path).
522+
assertNotNull(response);
523+
assertFalse(response.getColumns().isEmpty(), "the matching COMPLETE column should be returned");
524+
assertAllRowsHaveStatus(response, MetadataStatus.COMPLETE);
525+
assertTrue(
526+
response.getColumns().stream()
527+
.allMatch(c -> c.getColumnName().toLowerCase().contains("alpha")),
528+
"only columns whose name matches the pattern should be returned");
529+
assertEquals(
530+
response.getColumns().size(),
531+
response.getTotalUniqueColumns(),
532+
"totalUniqueColumns must not include pattern-mismatched columns");
533+
}
534+
535+
/**
536+
* Every returned row must carry the requested aggregate status — the core guarantee of #26824
537+
* (before the fix a COMPLETE/INCOMPLETE filter leaked rows of other statuses).
538+
*/
539+
private void assertAllRowsHaveStatus(ColumnGridResponse response, MetadataStatus expected) {
540+
for (ColumnGridItem item : response.getColumns()) {
541+
assertEquals(
542+
expected,
543+
item.getMetadataStatus(),
544+
"column '" + item.getColumnName() + "' should have status " + expected);
545+
}
425546
}
426547

427548
@Test
@@ -1424,6 +1545,7 @@ private DatabaseService createTableWithFullMetadata(TestNamespace ns) {
14241545
Columns.build("full_metadata_id")
14251546
.withType(ColumnDataType.BIGINT)
14261547
.withDescription("Primary key with description")
1548+
.withTags(List.of(new TagLabel().withTagFQN("PII.Sensitive")))
14271549
.create();
14281550

14291551
Tables.create()

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ColumnRepository.java

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import org.openmetadata.schema.EntityInterface;
4141
import org.openmetadata.schema.api.data.BulkColumnUpdatePreview;
4242
import org.openmetadata.schema.api.data.BulkColumnUpdateRequest;
43-
import org.openmetadata.schema.api.data.ColumnGridItem;
4443
import org.openmetadata.schema.api.data.ColumnGridResponse;
4544
import org.openmetadata.schema.api.data.ColumnMetadata;
4645
import org.openmetadata.schema.api.data.ColumnOccurrence;
@@ -97,39 +96,10 @@ public ColumnRepository(Authorizer authorizer, SearchClient searchClient) {
9796
public ColumnGridResponse getColumnGridPaginated(
9897
SecurityContext securityContext, ColumnAggregator.ColumnAggregationRequest request)
9998
throws IOException {
100-
ColumnGridResponse response = columnAggregator.aggregateColumns(request);
101-
102-
if (Boolean.TRUE.equals(request.getHasConflicts())) {
103-
response.setColumns(
104-
response.getColumns().stream()
105-
.filter(ColumnGridItem::getHasVariations)
106-
.collect(Collectors.toList()));
107-
}
108-
109-
if (Boolean.TRUE.equals(request.getHasMissingMetadata())) {
110-
response.setColumns(
111-
response.getColumns().stream()
112-
.filter(this::hasMissingMetadata)
113-
.collect(Collectors.toList()));
114-
}
115-
116-
// Filter by INCONSISTENT status (requires post-aggregation filtering)
117-
if ("INCONSISTENT".equalsIgnoreCase(request.getMetadataStatus())) {
118-
response.setColumns(
119-
response.getColumns().stream()
120-
.filter(ColumnGridItem::getHasVariations)
121-
.collect(Collectors.toList()));
122-
}
123-
124-
return response;
125-
}
126-
127-
private boolean hasMissingMetadata(ColumnGridItem item) {
128-
return item.getGroups().stream()
129-
.anyMatch(
130-
group ->
131-
(group.getDescription() == null || group.getDescription().isEmpty())
132-
|| (group.getTags() == null || group.getTags().isEmpty()));
99+
// Row-level filters (metadataStatus / hasConflicts / hasMissingMetadata) are applied inside the
100+
// aggregator over the fully-grouped columns, before pagination, so page counts and per-page
101+
// size stay correct (#26824). Nothing to post-process here.
102+
return columnAggregator.aggregateColumns(request);
133103
}
134104

135105
public Column getColumnByFQN(

openmetadata-service/src/main/java/org/openmetadata/service/resources/columns/ColumnResource.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,13 +319,15 @@ public Response getColumnGrid(
319319
boolean hasMissingMetadata,
320320
@Parameter(
321321
description =
322-
"Filter by metadata status: MISSING (no description AND no tags), "
322+
"Filter by aggregate metadata status of a column across all its occurrences: "
323+
+ "MISSING (no description AND no tags), "
323324
+ "INCOMPLETE (has description OR tags, but not both), "
324-
+ "COMPLETE (has both description AND tags)",
325+
+ "COMPLETE (has both description AND tags), "
326+
+ "INCONSISTENT (occurrences disagree on description/tags)",
325327
schema =
326328
@Schema(
327329
type = "string",
328-
allowableValues = {"MISSING", "INCOMPLETE", "COMPLETE"}))
330+
allowableValues = {"MISSING", "INCOMPLETE", "COMPLETE", "INCONSISTENT"}))
329331
@QueryParam("metadataStatus")
330332
String metadataStatus,
331333
@Parameter(

openmetadata-service/src/main/java/org/openmetadata/service/search/ColumnAggregator.java

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,13 @@
1616
import com.fasterxml.jackson.core.type.TypeReference;
1717
import java.io.IOException;
1818
import java.nio.charset.StandardCharsets;
19+
import java.util.ArrayList;
1920
import java.util.Base64;
21+
import java.util.Comparator;
2022
import java.util.List;
23+
import java.util.Locale;
2124
import java.util.Map;
25+
import org.openmetadata.schema.api.data.ColumnGridItem;
2226
import org.openmetadata.schema.api.data.ColumnGridResponse;
2327
import org.openmetadata.schema.utils.JsonUtils;
2428
import org.slf4j.Logger;
@@ -115,6 +119,107 @@ static int toIntSaturating(long value) {
115119
return (int) value;
116120
}
117121

122+
/**
123+
* True if a request carries a row-level filter — one that acts on the aggregate status of a
124+
* grouped column (metadataStatus, hasConflicts, hasMissingMetadata) rather than on individual
125+
* documents. These cannot be pushed into the search query (the aggregate status is only known
126+
* after grouping occurrences), so they are applied via {@link #paginateFilteredItems}.
127+
*/
128+
static boolean hasRowLevelFilter(ColumnAggregationRequest request) {
129+
return !nullOrEmptyStr(request.getMetadataStatus())
130+
|| Boolean.TRUE.equals(request.getHasConflicts())
131+
|| Boolean.TRUE.equals(request.getHasMissingMetadata());
132+
}
133+
134+
/** True if the grouped column satisfies every active row-level filter on the request. */
135+
static boolean matchesRowFilters(ColumnGridItem item, ColumnAggregationRequest request) {
136+
if (Boolean.TRUE.equals(request.getHasConflicts())
137+
&& !Boolean.TRUE.equals(item.getHasVariations())) {
138+
return false;
139+
}
140+
if (Boolean.TRUE.equals(request.getHasMissingMetadata()) && !itemHasMissingMetadata(item)) {
141+
return false;
142+
}
143+
String status = request.getMetadataStatus();
144+
if (!nullOrEmptyStr(status)) {
145+
String itemStatus = item.getMetadataStatus() != null ? item.getMetadataStatus().value() : "";
146+
return status.trim().equalsIgnoreCase(itemStatus);
147+
}
148+
return true;
149+
}
150+
151+
/**
152+
* Drop columns whose name doesn't contain the request's {@code columnNamePattern}
153+
* (case-insensitive). The name wildcard in the search query only scopes which entities are
154+
* scanned; flat-object mapping can't isolate the matching column, so the pattern is enforced per
155+
* column here. Shared by the ES and OS row-filter scans so their pattern semantics can't drift.
156+
*/
157+
static void applyColumnNamePattern(
158+
Map<String, ?> columnsByName, ColumnAggregationRequest request) {
159+
if (nullOrEmptyStr(request.getColumnNamePattern())) {
160+
return;
161+
}
162+
String pattern = request.getColumnNamePattern().toLowerCase(Locale.ROOT);
163+
columnsByName.keySet().removeIf(name -> !name.toLowerCase(Locale.ROOT).contains(pattern));
164+
}
165+
166+
/** A column has missing metadata if any of its groups lacks a description or tags. */
167+
static boolean itemHasMissingMetadata(ColumnGridItem item) {
168+
if (item.getGroups() == null) {
169+
return true;
170+
}
171+
return item.getGroups().stream()
172+
.anyMatch(
173+
group ->
174+
(group.getDescription() == null || group.getDescription().isEmpty())
175+
|| (group.getTags() == null || group.getTags().isEmpty()));
176+
}
177+
178+
/**
179+
* Apply row-level filters to the fully-grouped column list and paginate the result in memory.
180+
* Filtering the aggregate items (not documents) is what makes a "Complete"/"Incomplete"/… filter
181+
* return only rows whose displayed status matches, and computing totals from the filtered set is
182+
* what keeps the page count and per-page size correct (issue #26824). Ordering is by column name
183+
* (case-insensitive) so the offset cursor is stable across pages.
184+
*/
185+
static ColumnGridResponse paginateFilteredItems(
186+
List<ColumnGridItem> allItems, ColumnAggregationRequest request) {
187+
List<ColumnGridItem> filtered =
188+
allItems.stream()
189+
.filter(item -> matchesRowFilters(item, request))
190+
.sorted(
191+
Comparator.comparing(
192+
ColumnGridItem::getColumnName,
193+
Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER)))
194+
.toList();
195+
196+
int totalUniqueColumns = filtered.size();
197+
int totalOccurrences =
198+
filtered.stream()
199+
.mapToInt(item -> item.getTotalOccurrences() != null ? item.getTotalOccurrences() : 0)
200+
.sum();
201+
202+
int offset = decodeSearchOffset(request.getCursor());
203+
int pageSize = request.getSize();
204+
int fromIndex = Math.min(offset, totalUniqueColumns);
205+
int toIndex = Math.min(offset + pageSize, totalUniqueColumns);
206+
207+
List<ColumnGridItem> page = new ArrayList<>(filtered.subList(fromIndex, toIndex));
208+
boolean hasMore = toIndex < totalUniqueColumns;
209+
String cursor = hasMore ? encodeSearchOffset(toIndex) : null;
210+
211+
ColumnGridResponse response = new ColumnGridResponse();
212+
response.setColumns(page);
213+
response.setTotalUniqueColumns(totalUniqueColumns);
214+
response.setTotalOccurrences(totalOccurrences);
215+
response.setCursor(cursor);
216+
return response;
217+
}
218+
219+
private static boolean nullOrEmptyStr(String s) {
220+
return s == null || s.isBlank();
221+
}
222+
118223
/** Phase 1 result: matching column names and the total doc_count summed across buckets. */
119224
record NamesWithCount(List<String> names, long totalDocCount) {}
120225

0 commit comments

Comments
 (0)