Skip to content

fix: server-side column tag filtering across all pages (#25063) #27334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
david-mamani wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: server-side column tag filtering across all pages (#25063) #27334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d36d4b
fix: server-side column tag filtering across all pages (#25063)
david-mamani Apr 14, 2026
1efa96a
Merge branch 'main' into fix/25063-column-tag-filter-across-all-pages
david-mamani Apr 14, 2026
4b396a9
style: apply spotless formatting to pass Java checkstyle
david-mamani Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 184 additions & 11 deletions openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@
import org.openmetadata.service.util.EntityUtil.RelationIncludes;
import org.openmetadata.service.util.FullyQualifiedName;
import org.openmetadata.service.util.RestUtil;
import org.openmetadata.service.util.TableUtil;
import org.openmetadata.service.util.ValidatorUtil;

@Slf4j
Expand Down Expand Up @@ -2785,6 +2786,32 @@ public ResultList<Column> getTableColumns(
List<EntityReference> piiOwners,
Authorizer authorizer,
SecurityContext securityContext) {
return getTableColumns(
tableId,
limit,
offset,
fieldsParam,
include,
sortBy,
sortOrder,
null,
piiOwners,
authorizer,
securityContext);
}

public ResultList<Column> getTableColumns(
UUID tableId,
int limit,
int offset,
String fieldsParam,
Include include,
String sortBy,
String sortOrder,
String tag,
List<EntityReference> piiOwners,
Authorizer authorizer,
SecurityContext securityContext) {
Table table = find(tableId, include);
return getTableColumnsInternal(
table,
Expand All @@ -2794,6 +2821,7 @@ public ResultList<Column> getTableColumns(
include,
sortBy,
sortOrder,
tag,
piiOwners,
authorizer,
securityContext);
Expand Down Expand Up @@ -2858,6 +2886,32 @@ public ResultList<Column> getTableColumnsByFQN(
List<EntityReference> piiOwners,
Authorizer authorizer,
SecurityContext securityContext) {
return getTableColumnsByFQN(
fqn,
limit,
offset,
fieldsParam,
include,
sortBy,
sortOrder,
null,
piiOwners,
authorizer,
securityContext);
}

public ResultList<Column> getTableColumnsByFQN(
String fqn,
int limit,
int offset,
String fieldsParam,
Include include,
String sortBy,
String sortOrder,
String tag,
List<EntityReference> piiOwners,
Authorizer authorizer,
SecurityContext securityContext) {
Table table = findByName(fqn, include);
return getTableColumnsInternal(
table,
Expand All @@ -2867,6 +2921,7 @@ public ResultList<Column> getTableColumnsByFQN(
include,
sortBy,
sortOrder,
tag,
piiOwners,
authorizer,
securityContext);
Expand All @@ -2880,6 +2935,7 @@ private ResultList<Column> getTableColumnsInternal(
Include include,
String sortBy,
String sortOrder,
String tag,
List<EntityReference> piiOwners,
Authorizer authorizer,
SecurityContext securityContext) {
Expand All @@ -2889,8 +2945,26 @@ private ResultList<Column> getTableColumnsInternal(
return new ResultList<>(new ArrayList<>(), "0", String.valueOf(offset + limit), 0);
}

boolean hasTagFilter = tag != null && !tag.trim().isEmpty();
boolean needsTags = fieldsParam != null && fieldsParam.contains("tags");

// When filtering by tag, populate tags on ALL columns BEFORE filtering
if (hasTagFilter && needsTags) {
Comment on lines +2949 to +2952
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasTagFilter currently only triggers populateEntityFieldTags(...) when needsTags is true (i.e., when the caller requested tags). If a client calls the endpoint with tag=... but without fields including tags (or with fields=*), filtering will run against column.getTags() that were never populated and can incorrectly return 0 columns (and may also break PII masking when profile is requested). Consider always populating tags when tag is provided (for filtering purposes), and if tags were not requested, strip them from the response after filtering/pagination to preserve the fields contract.

Suggested change
boolean needsTags = fieldsParam != null && fieldsParam.contains("tags");
// When filtering by tag, populate tags on ALL columns BEFORE filtering
if (hasTagFilter && needsTags) {
boolean requestedTags =
fieldsParam != null && (fieldsParam.contains("tags") || fieldsParam.contains("*"));
boolean needsTags = requestedTags || hasTagFilter;
// Populate tags before any logic that depends on them, including tag filtering.
if (needsTags) {

Copilot uses AI. Check for mistakes.
populateEntityFieldTags(entityType, allColumns, table.getFullyQualifiedName(), true);
}
Comment on lines +2948 to +2954
Copy link
Copy Markdown

@gitar-bot gitar-bot Bot Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: Tag filter silently returns empty when fields omits 'tags'

In both getTableColumnsInternal and searchTableColumnsInternal, tags are only populated on columns when hasTagFilter && needsTags is true (lines 2917 and 3295). However, the tag filter is applied whenever hasTagFilter is true (lines 2924 and 3323), regardless of whether tags were populated. If a caller passes tag=PII.Sensitive but doesn't include tags in the fields parameter, column.getTags() will be null, columnMatchesAnyTag will always return false, and the API will silently return zero results.

While the current frontend always sends fields: 'tags,...', the backend API is public and should be defensive. Another caller (or a future frontend change) could easily trigger this.

The fix is to populate tags whenever a tag filter is active, regardless of the fields parameter.

Suggested fix:

// In getTableColumnsInternal (and same pattern in searchTableColumnsInternal):
boolean hasTagFilter = tag != null && !tag.trim().isEmpty();
boolean needsTags = fieldsParam != null && fieldsParam.contains("tags");

// When filtering by tag, ALWAYS populate tags, even if not requested in fields
if (hasTagFilter) {
  populateEntityFieldTags(entityType, allColumns, table.getFullyQualifiedName(), true);
}

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

Comment on lines +2948 to +2954
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tag filtering depends on needsTags (fields containing tags). If a caller supplies the tag query param but does not request tags (or uses fields=*), tags may not be populated and the filter will incorrectly return zero results. Populate tags whenever hasTagFilter is true (independent of requested fields), and if tags weren’t requested, strip them from the response before returning to preserve the existing API contract.

Copilot uses AI. Check for mistakes.

List<Column> workingColumns = new ArrayList<>(allColumns);

// Apply tag filter if provided
if (hasTagFilter) {
Set<String> tagFQNs = Set.of(tag.split(","));
workingColumns =
workingColumns.stream()
.filter(column -> columnMatchesAnyTag(column, tagFQNs))
.collect(Collectors.toCollection(ArrayList::new));
Comment on lines +2960 to +2964
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set.of(tag.split(",")) will throw IllegalArgumentException if the tag list contains duplicates (or can include untrimmed/blank entries). Consider parsing with trimming + blank filtering + Collectors.toSet() to avoid 500s from duplicate/whitespace input.

Suggested change
Set<String> tagFQNs = Set.of(tag.split(","));
workingColumns =
workingColumns.stream()
.filter(column -> columnMatchesAnyTag(column, tagFQNs))
.collect(Collectors.toCollection(ArrayList::new));
Set<String> tagFQNs =
Arrays.stream(tag.split(","))
.map(String::trim)
.filter(Predicate.not(String::isBlank))
.collect(Collectors.toSet());
if (!tagFQNs.isEmpty()) {
workingColumns =
workingColumns.stream()
.filter(column -> columnMatchesAnyTag(column, tagFQNs))
.collect(Collectors.toCollection(ArrayList::new));
}

Copilot uses AI. Check for mistakes.
Comment on lines +2959 to +2964
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set.of(tag.split(",")) will throw IllegalArgumentException if the query param contains duplicate tag FQNs (e.g., tag=a,b,a), and it also preserves leading/trailing whitespace (e.g., "PII.Sensitive, Tier.Tier1"). Parse the CSV by trimming each entry, dropping blanks, and collecting via Collectors.toSet() (or similar) so duplicates/whitespace don’t cause 500s.

Copilot uses AI. Check for mistakes.
}

// Sort columns based on sortBy and sortOrder parameters
List<Column> sortedColumns = new ArrayList<>(allColumns);
Comparator<Column> comparator;
if ("ordinalPosition".equals(sortBy)) {
comparator =
Expand All @@ -2907,17 +2981,18 @@ private ResultList<Column> getTableColumnsInternal(
if ("desc".equalsIgnoreCase(sortOrder)) {
comparator = comparator.reversed();
}
sortedColumns.sort(comparator);
workingColumns.sort(comparator);

// Apply pagination
int total = sortedColumns.size();
int total = workingColumns.size();
int fromIndex = Math.min(offset, total);
int toIndex = Math.min(offset + limit, total);

List<Column> paginatedColumns = sortedColumns.subList(fromIndex, toIndex);
List<Column> paginatedColumns = workingColumns.subList(fromIndex, toIndex);

// Apply field processing if needed
if (fieldsParam != null && fieldsParam.contains("tags")) {
// Populate tags only if NOT already done for tag filtering
if (needsTags && !hasTagFilter) {
populateEntityFieldTags(entityType, paginatedColumns, table.getFullyQualifiedName(), true);
}

Expand All @@ -2935,7 +3010,9 @@ private ResultList<Column> getTableColumnsInternal(

if (fieldsParam != null && fieldsParam.contains("profile")) {
setColumnProfile(paginatedColumns);
populateEntityFieldTags(entityType, paginatedColumns, table.getFullyQualifiedName(), true);
if (!hasTagFilter) {
populateEntityFieldTags(entityType, paginatedColumns, table.getFullyQualifiedName(), true);
}
paginatedColumns =
piiOwners != null
? PIIMasker.getTableProfile(piiOwners, paginatedColumns, authorizer, securityContext)
Expand Down Expand Up @@ -3149,9 +3226,44 @@ public ResultList<Column> searchTableColumnsById(
String sortOrder,
Authorizer authorizer,
SecurityContext securityContext) {
return searchTableColumnsById(
id,
query,
limit,
offset,
fieldsParam,
include,
sortBy,
sortOrder,
null,
authorizer,
securityContext);
}

public ResultList<Column> searchTableColumnsById(
UUID id,
String query,
int limit,
int offset,
String fieldsParam,
Include include,
String sortBy,
String sortOrder,
String tag,
Authorizer authorizer,
SecurityContext securityContext) {
Table table = get(null, id, getFields(fieldsParam), include, false);
return searchTableColumnsInternal(
table, query, limit, offset, fieldsParam, sortBy, sortOrder, authorizer, securityContext);
table,
query,
limit,
offset,
fieldsParam,
sortBy,
sortOrder,
tag,
authorizer,
securityContext);
}

public ResultList<Column> searchTableColumnsByFQN(
Expand Down Expand Up @@ -3187,9 +3299,44 @@ public ResultList<Column> searchTableColumnsByFQN(
String sortOrder,
Authorizer authorizer,
SecurityContext securityContext) {
return searchTableColumnsByFQN(
fqn,
query,
limit,
offset,
fieldsParam,
include,
sortBy,
sortOrder,
null,
authorizer,
securityContext);
}

public ResultList<Column> searchTableColumnsByFQN(
String fqn,
String query,
int limit,
int offset,
String fieldsParam,
Include include,
String sortBy,
String sortOrder,
String tag,
Authorizer authorizer,
SecurityContext securityContext) {
Table table = getByName(null, fqn, getFields(fieldsParam), include, false);
return searchTableColumnsInternal(
table, query, limit, offset, fieldsParam, sortBy, sortOrder, authorizer, securityContext);
table,
query,
limit,
offset,
fieldsParam,
sortBy,
sortOrder,
tag,
authorizer,
securityContext);
}

private ResultList<Column> searchTableColumnsInternal(
Expand All @@ -3200,13 +3347,23 @@ private ResultList<Column> searchTableColumnsInternal(
String fieldsParam,
String sortBy,
String sortOrder,
String tag,
Authorizer authorizer,
SecurityContext securityContext) {
List<Column> allColumns = table.getColumns();
if (allColumns == null || allColumns.isEmpty()) {
return new ResultList<>(List.of(), null, null, 0);
}

boolean hasTagFilter = tag != null && !tag.trim().isEmpty();
Fields fields = getFields(fieldsParam);
boolean needsTags = fields.contains("tags") || fields.contains("*");

// When filtering by tag, populate tags on ALL columns BEFORE filtering
if (hasTagFilter && needsTags) {
Comment on lines +3362 to +3363
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the non-search endpoint: tag filtering is gated on needsTags, so tag queries can silently return no results when fields doesn’t include tags (or *). The server-side filter should populate tags whenever tag is provided, independent of the requested return fields.

Suggested change
// When filtering by tag, populate tags on ALL columns BEFORE filtering
if (hasTagFilter && needsTags) {
// Populate tags when they are needed either for server-side filtering or for the response.
if (hasTagFilter || needsTags) {

Copilot uses AI. Check for mistakes.
Comment on lines +3362 to +3363
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasTagFilter is currently gated by needsTags before calling populateEntityFieldTags(...). If a client uses tag=... but does not request tags (or requests fields=*), columnMatchesAnyTag() will evaluate against null/unpopulated tags and can filter out everything. Tag filtering should work regardless of which fields are requested; consider always populating tags when tag is present (and optionally stripping tags from the response when not requested).

Suggested change
// When filtering by tag, populate tags on ALL columns BEFORE filtering
if (hasTagFilter && needsTags) {
// Populate tags before filtering or when tags are explicitly requested in the response
if (hasTagFilter || needsTags) {

Copilot uses AI. Check for mistakes.
populateEntityFieldTags(entityType, allColumns, table.getFullyQualifiedName(), true);
}

// Flatten nested columns for search
List<Column> flattenedColumns = flattenTableColumns(allColumns);

Expand All @@ -3230,6 +3387,15 @@ private ResultList<Column> searchTableColumnsInternal(
.toList());
}

// Apply tag filter if provided
if (hasTagFilter) {
Set<String> tagFQNs = Set.of(tag.split(","));
matchingColumns =
matchingColumns.stream()
.filter(column -> columnMatchesAnyTag(column, tagFQNs))
.collect(Collectors.toCollection(ArrayList::new));
Comment on lines +3390 to +3396
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same parsing issue here: Set.of(tag.split(",")) can throw on duplicates and won’t trim whitespace. Use a trim + blank-filter + toSet() approach so user-supplied query params can’t trigger 500s.

Copilot uses AI. Check for mistakes.
}
Comment on lines +3390 to +3397
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set.of(tag.split(",")) is also used here and has the same failure mode: duplicate values will throw at runtime, and whitespace/empty segments won’t be normalized. Parse with trimming + blank filtering + Collectors.toSet() for robustness.

Copilot uses AI. Check for mistakes.

// Sort matching columns based on sortBy and sortOrder parameters
Comparator<Column> comparator;
if ("ordinalPosition".equals(sortBy)) {
Expand All @@ -3256,20 +3422,22 @@ private ResultList<Column> searchTableColumnsInternal(
List<Column> paginatedResults =
startIndex < total ? matchingColumns.subList(startIndex, endIndex) : List.of();

Fields fields = getFields(fieldsParam);
if (fields.contains("customMetrics") || fields.contains("*")) {
for (Column column : paginatedResults) {
column.setCustomMetrics(getCustomMetrics(table, column.getName()));
}
}

if (fields.contains("tags") || fields.contains("*")) {
// Populate tags only if NOT already done for tag filtering
if (needsTags && !hasTagFilter) {
populateEntityFieldTags(entityType, paginatedResults, table.getFullyQualifiedName(), true);
}

if (fieldsParam != null && fieldsParam.contains("profile")) {
setColumnProfile(matchingColumns);
populateEntityFieldTags(entityType, matchingColumns, table.getFullyQualifiedName(), true);
if (!hasTagFilter) {
populateEntityFieldTags(entityType, matchingColumns, table.getFullyQualifiedName(), true);
}
matchingColumns =
PIIMasker.getTableProfile(
table.getFullyQualifiedName(), matchingColumns, authorizer, securityContext);
Expand All @@ -3280,6 +3448,11 @@ private ResultList<Column> searchTableColumnsInternal(
return new ResultList<>(paginatedResults, before, after, total);
}

/** Check if a column or any of its children match any of the given tag FQNs. */
private boolean columnMatchesAnyTag(Column column, Set<String> tagFQNs) {
return TableUtil.columnMatchesAnyTag(column, tagFQNs);
}

private List<Column> flattenTableColumns(List<Column> columns) {
List<Column> flattened = new ArrayList<>();
for (Column column : columns) {
Expand Down
Loading
Loading