Skip to content

Fixes #25063: Add server-side tag filtering for table columns via ColumnResource API#27331

Open
mohitjeswani01 wants to merge 4 commits intoopen-metadata:mainfrom
mohitjeswani01:fix/25063-column-tag-filter-cross-page
Open

Fixes #25063: Add server-side tag filtering for table columns via ColumnResource API#27331
mohitjeswani01 wants to merge 4 commits intoopen-metadata:mainfrom
mohitjeswani01:fix/25063-column-tag-filter-cross-page

Conversation

@mohitjeswani01
Copy link
Copy Markdown

@mohitjeswani01 mohitjeswani01 commented Apr 14, 2026

Description:

Fixes #25063

Problem:
Column tag filters in the table schema view only filtered columns
visible on the current page (max 50 columns). Tables with 80+ columns
across multiple pages showed incorrect filter results — columns on
other pages with matching tags were completely invisible to the filter.

Root Cause:
Tag filtering was applied client-side against tableColumns state
which only holds the current paginated slice. The filter had no
awareness of columns on other pages.

Solution:
Implemented server-side tag filtering at the ColumnResource API level
as suggested by @harshach. The filter is now delegated to the backend
which applies it across all columns before paginating the response.

Backend changes (TableResource.java, TableRepository.java):

  • Added tags query parameter (comma-separated tag FQNs) to both
    searchTableColumnsByFQN and searchTableColumnsById endpoints
  • Implemented tag filtering logic inside searchTableColumnsInternal
    using OR logic — applied after name/description filtering and before
    pagination
  • Null-safe implementation — existing behaviour unchanged when no
    tags parameter is provided

Frontend changes (SchemaTable.component.tsx, tableAPI.ts):

  • Added tags?: string to GetTableColumnsParams and
    SearchTableColumnsParams interfaces
  • Extracted buildTagsParam() utility function to build comma-separated
    tag string from active Classification and Glossary filters
  • Both fetchTableColumns and searchTableColumns now pass the tags
    parameter to the backend API call
  • tagFilter added to useCallback dependency arrays to ensure
    refetch on filter change

Tests (SchemaTable.test.tsx):

  • Added 5 unit tests under
    Tag Filtering Server-Side Integration (Issue #25063)
  • Tests cover: no tags on mount, Classification filter, Glossary filter,
    multiple tags comma-separated, and empty filter returns undefined
  • Tests verify buildTagsParam pure function directly for reliability

Test Results:

Screenshot 1 — Tests starting:
image

Screenshot 2 — Test results (21/21 passed):
image

Note on full-stack testing:
Local full-stack execution was not possible on this development machine
(8GB RAM) as OpenMetadata requires ~8GB for backend + Elasticsearch +
MySQL simultaneously. The fix has been validated through unit tests
covering the core tag parameter building and API integration logic.
Backend logic correctness is verified through code review of the
searchTableColumnsInternal filtering implementation.


Type of change:

  • Bug fix

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes #25063: Add server-side tag filtering for table columns via ColumnResource API
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added a test that covers the exact scenario we are fixing.

@mohitjeswani01 mohitjeswani01 requested a review from a team as a code owner April 14, 2026 00:01
Copilot AI review requested due to automatic review settings April 14, 2026 00:01
@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to fix Issue #25063 by moving column tag filtering from client-side (current-page only) to server-side filtering so tag filters work across all table columns before pagination.

Changes:

  • Added a tags query parameter to the backend table column search endpoints and implemented tag-based filtering in TableRepository.searchTableColumnsInternal.
  • Updated UI table column fetch/search requests to pass a tags param and introduced a buildTagsParam helper.
  • Added unit tests for tag-param construction and (partial) API integration behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
openmetadata-ui/src/main/resources/ui/src/rest/tableAPI.ts Adds tags to table column request parameter typings.
openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx Adds buildTagsParam and attempts to send tags to backend; updates hook dependencies.
openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.test.tsx Adds tests for tag param building and API-call expectations.
openmetadata-service/src/main/java/org/openmetadata/service/resources/databases/TableResource.java Adds tags query param to /columns/search endpoints.
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java Implements server-side tag filtering inside column search logic.

Comment on lines +229 to +236
const tagFilter = useMemo(() => {
const tags = getAllTags(tableColumns);

return groupBy(uniqBy(tags, 'value'), (tag) => tag.source) as Record<
TagSource,
TagFilterOptions[]
>;
}, [tableColumns]);
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.

tagFilter is derived from tableColumns via getAllTags(...), i.e., it's the set of available tag filter options for the currently loaded page. Using it to build the tags query param will send all tags from the current page (and will change as pagination changes), rather than the user’s selected tag filters. This breaks the intended behavior and can also cause repeated refetching because tagFilter changes whenever tableColumns changes. Introduce separate state for the active/selected tag filters (e.g., from AntD Table onChange filters or a dedicated filter state), keep tagFilter only for options, and build the server tags param from the selected values instead.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed — Removed duplicate tags?: string from
SearchTableColumnsParams since it already inherits from
GetTableColumnsParams.

Comment on lines 346 to 366
useEffect(() => {
if (searchText) {
searchTableColumns(searchText, currentPage, sortBy, sortOrder);
}
}, [searchText, currentPage, searchTableColumns, sortBy, sortOrder]);
}, [searchText, currentPage, searchTableColumns, sortBy, sortOrder, tagFilter]);

useEffect(() => {
if (searchText) {
return;
}
fetchTableColumns(currentPage, sortBy, sortOrder);
}, [
tableFqn,
pageSize,
currentPage,
searchText,
fetchTableColumns,
sortBy,
sortOrder,
tagFilter,
]);
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.

Including tagFilter in these effects’ dependency arrays will likely create a fetch loop: fetchTableColumns/searchTableColumns update tableColumnstagFilter recomputes (depends on tableColumns) → effect reruns → triggers another fetch, etc. The dependency for refetching should be the selected tag filters (a stable piece of state), not the derived tag option list computed from fetched data.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed — fetchTableColumns now calls searchTableColumnsByFQN
with q: '' when tag filters are active, ensuring server-side
tag filtering applies in both search and non-search scenarios.

include?: Include;
sortBy?: 'name' | 'ordinalPosition';
sortOrder?: 'asc' | 'desc';
tags?: string;
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.

Adding tags?: string to GetTableColumnsParams implies the /tables/.../columns endpoints support tag filtering, but TableResource#getTableColumns/getTableColumnsByFQN currently do not accept a tags query param. This can mislead callers and silently do nothing at runtime. Either implement tags on the non-search column list endpoints, or restrict tags to SearchTableColumnsParams only and ensure the UI calls the /columns/search endpoints when tag filters are active.

Suggested change
tags?: string;

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed — Removed unused TagFilterOptions and TagSource imports.
buildTagsParam now accepts Record<string, string[]> directly,
eliminating the need for these type imports in tests.

Comment on lines +3239 to +3253
if (tags != null && !tags.trim().isEmpty()) {
String[] tagFQNs = tags.split(",");
matchingColumns =
new ArrayList<>(
matchingColumns.stream()
.filter(
column ->
column.getTags() != null
&& column.getTags().stream()
.anyMatch(
tag ->
Arrays.stream(tagFQNs)
.anyMatch(
tagFQN -> tag.getTagFQN().equals(tagFQN.trim()))))
.toList());
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.

Current tag filtering iterates Arrays.stream(tagFQNs) for every tag on every column, which is avoidable overhead for wide tables. Consider normalizing the incoming tags once (trim, drop empties) into a Set<String> and then checking membership (set.contains(tag.getTagFQN())) during the stream filter. This keeps the same OR semantics but reduces per-column work.

Copilot uses AI. Check for mistakes.
Comment on lines +2009 to +2014
String sortOrder,
@Parameter(
description = "Comma-separated list of tag FQNs to filter columns by",
schema = @Schema(type = "string"))
@QueryParam("tags")
String tags) {
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.

tags is only wired into the /columns/search endpoints here, but the UI’s initial (non-search) load uses /tables/.../columns. Without adding tags to the non-search /columns endpoints (or updating the UI to call /columns/search when tag filters are active), server-side tag filtering won’t apply in the common no-search-text case.

Copilot uses AI. Check for mistakes.
Comment on lines 301 to 308
const response = await getTableColumnsByFQN(tableFqn, {
limit: pageSize,
offset: offset,
fields: 'tags,customMetrics,extension',
sortBy: sortByParam,
sortOrder: sortOrderParam,
...(tagsParam ? { tags: tagsParam } : {}),
});
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.

getTableColumnsByFQN calls /tables/name/{fqn}/columns, but the backend getTableColumnsByFQN endpoint in TableResource does not accept a tags query parameter (only the /columns/search endpoints do). Passing tags here will be ignored server-side, so tag filtering won’t work when there’s no search query. Either add tags support to the non-search /columns endpoints (and their repository methods), or switch the UI to call the /columns/search endpoint when tag filters are active (even if q is empty).

Suggested change
const response = await getTableColumnsByFQN(tableFqn, {
limit: pageSize,
offset: offset,
fields: 'tags,customMetrics,extension',
sortBy: sortByParam,
sortOrder: sortOrderParam,
...(tagsParam ? { tags: tagsParam } : {}),
});
const response = tagsParam
? await searchTableColumnsByFQN(tableFqn, {
q: '',
limit: pageSize,
offset: offset,
fields: 'tags,customMetrics,extension',
sortBy: sortByParam,
sortOrder: sortOrderParam,
tags: tagsParam,
})
: await getTableColumnsByFQN(tableFqn, {
limit: pageSize,
offset: offset,
fields: 'tags,customMetrics,extension',
sortBy: sortByParam,
sortOrder: sortOrderParam,
});

Copilot uses AI. Check for mistakes.
Comment on lines +19 to 25
import { getTableColumnsByFQN, searchTableColumnsByFQN } from '../../../rest/tableAPI';
import { DEFAULT_ENTITY_PERMISSION } from '../../../utils/PermissionsUtils';
import SchemaTable from './SchemaTable.component';
import SchemaTable, { buildTagsParam } from './SchemaTable.component';
import { TagFilterOptions } from 'Models';
import { TagSource } from '../../../generated/type/schema';
import { getAllTags, searchTagInData } from '../../../utils/TableTags/TableTags.utils';

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.

The new test suite imports getAllTags/searchTagInData but never uses them, which will trip lint rules like no-unused-vars in CI. Remove the unused imports (or use them if needed for assertions). Also consider moving the Models import up with other external imports to match the repo’s organized-imports expectations.

Copilot uses AI. Check for mistakes.
Comment on lines +619 to +670
beforeEach(() => {
jest.clearAllMocks();

// Reset core mocks back to default implementation for TEST 1 integration
(getTableColumnsByFQN as jest.Mock).mockResolvedValue({
data: mockColumns,
paging: { total: mockColumns.length },
});
(searchTableColumnsByFQN as jest.Mock).mockResolvedValue({
data: mockColumns,
paging: { total: mockColumns.length },
});
});

it('TEST 1 — No tags param sent when no filter active', async () => {

await act(async () => {
render(<SchemaTable />, { wrapper: MemoryRouter });
});

expect(getTableColumnsByFQN).toHaveBeenCalledWith(
MOCK_TABLE.fullyQualifiedName,
expect.not.objectContaining({ tags: expect.anything() })
);
});

it('TEST 2 — Classification filter', () => {
expect(buildTagsParam({
Classification: [{ value: 'PII.Sensitive' }],
Glossary: []
} as Record<TagSource, TagFilterOptions[]>)).toBe('PII.Sensitive');
});

it('TEST 3 — Glossary filter', () => {
expect(buildTagsParam({
Classification: [],
Glossary: [{ value: 'Glossary.Term1' }]
} as Record<TagSource, TagFilterOptions[]>)).toBe('Glossary.Term1');
});

it('TEST 4 — Multiple tags comma-separated', () => {
expect(buildTagsParam({
Classification: [{ value: 'PII.Sensitive' }],
Glossary: [{ value: 'Glossary.Term1' }]
} as Record<TagSource, TagFilterOptions[]>)).toBe('PII.Sensitive,Glossary.Term1');
});

it('TEST 5 — Empty filter returns undefined', () => {
expect(buildTagsParam({
Classification: [],
Glossary: []
} as Record<TagSource, TagFilterOptions[]>)).toBeUndefined();
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.

TEST 1 — No tags param sent when no filter active currently passes because getAllTags is mocked with no return value, so tagFilter is empty and buildTagsParam(tagFilter) becomes undefined. This doesn’t exercise the real-world case where columns have tags but the user has not selected any filters (which is the critical scenario for Issue #25063). Update the test to simulate columns that contain tags and verify that the API call only includes tags when the user actually applies a tag filter (once the component tracks selected filters separately).

Suggested change
beforeEach(() => {
jest.clearAllMocks();
// Reset core mocks back to default implementation for TEST 1 integration
(getTableColumnsByFQN as jest.Mock).mockResolvedValue({
data: mockColumns,
paging: { total: mockColumns.length },
});
(searchTableColumnsByFQN as jest.Mock).mockResolvedValue({
data: mockColumns,
paging: { total: mockColumns.length },
});
});
it('TEST 1 — No tags param sent when no filter active', async () => {
await act(async () => {
render(<SchemaTable />, { wrapper: MemoryRouter });
});
expect(getTableColumnsByFQN).toHaveBeenCalledWith(
MOCK_TABLE.fullyQualifiedName,
expect.not.objectContaining({ tags: expect.anything() })
);
});
it('TEST 2 — Classification filter', () => {
expect(buildTagsParam({
Classification: [{ value: 'PII.Sensitive' }],
Glossary: []
} as Record<TagSource, TagFilterOptions[]>)).toBe('PII.Sensitive');
});
it('TEST 3 — Glossary filter', () => {
expect(buildTagsParam({
Classification: [],
Glossary: [{ value: 'Glossary.Term1' }]
} as Record<TagSource, TagFilterOptions[]>)).toBe('Glossary.Term1');
});
it('TEST 4 — Multiple tags comma-separated', () => {
expect(buildTagsParam({
Classification: [{ value: 'PII.Sensitive' }],
Glossary: [{ value: 'Glossary.Term1' }]
} as Record<TagSource, TagFilterOptions[]>)).toBe('PII.Sensitive,Glossary.Term1');
});
it('TEST 5 — Empty filter returns undefined', () => {
expect(buildTagsParam({
Classification: [],
Glossary: []
} as Record<TagSource, TagFilterOptions[]>)).toBeUndefined();
const taggedColumns = [
{
...mockColumns[0],
tags: [
{
tagFQN: 'PII.Sensitive',
source: TagSource.Classification,
},
],
},
...mockColumns.slice(1),
] as Column[];
beforeEach(() => {
jest.clearAllMocks();
// Reset core mocks back to a realistic integration scenario where
// columns already contain tags, but no tag filter has been selected.
(getTableColumnsByFQN as jest.Mock).mockResolvedValue({
data: taggedColumns,
paging: { total: taggedColumns.length },
});
(searchTableColumnsByFQN as jest.Mock).mockResolvedValue({
data: taggedColumns,
paging: { total: taggedColumns.length },
});
(getAllTags as jest.Mock).mockReturnValue({
Classification: [{ value: 'PII.Sensitive', label: 'PII.Sensitive' }],
Glossary: [],
} as Record<TagSource, TagFilterOptions[]>);
(searchTagInData as jest.Mock).mockReturnValue(taggedColumns);
});
it('TEST 1 — No tags param sent when no filter active', async () => {
await act(async () => {
render(<SchemaTable />, { wrapper: MemoryRouter });
});
expect(getAllTags).toHaveBeenCalled();
expect(getTableColumnsByFQN).toHaveBeenCalled();
const [, requestParams] = (getTableColumnsByFQN as jest.Mock).mock.calls[0];
expect(requestParams).not.toHaveProperty('tags');
});
it('TEST 2 — Classification filter', () => {
expect(
buildTagsParam({
Classification: [{ value: 'PII.Sensitive' }],
Glossary: [],
} as Record<TagSource, TagFilterOptions[]>)
).toBe('PII.Sensitive');
});
it('TEST 3 — Glossary filter', () => {
expect(
buildTagsParam({
Classification: [],
Glossary: [{ value: 'Glossary.Term1' }],
} as Record<TagSource, TagFilterOptions[]>)
).toBe('Glossary.Term1');
});
it('TEST 4 — Multiple tags comma-separated', () => {
expect(
buildTagsParam({
Classification: [{ value: 'PII.Sensitive' }],
Glossary: [{ value: 'Glossary.Term1' }],
} as Record<TagSource, TagFilterOptions[]>)
).toBe('PII.Sensitive,Glossary.Term1');
});
it('TEST 5 — Empty filter returns undefined', () => {
expect(
buildTagsParam({
Classification: [],
Glossary: [],
} as Record<TagSource, TagFilterOptions[]>)
).toBeUndefined();

Copilot uses AI. Check for mistakes.

export interface SearchTableColumnsParams extends GetTableColumnsParams {
q?: string; // Search query
tags?: string;
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.

SearchTableColumnsParams already extends GetTableColumnsParams, which now includes tags?: string. Re-declaring tags?: string here is redundant and can lead to confusion about which params are supported. Remove the duplicate tags property from SearchTableColumnsParams (or, if only search should support tags, keep it only on the search params and not on GetTableColumnsParams).

Suggested change
tags?: string;

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@mohitjeswani01
Copy link
Copy Markdown
Author

Hi @harshach @PubChimps 👋

All review comments from Gitar Bot and GitHub Copilot have been
addressed in the latest push:

✅ Infinite render loop fixed — separate activeTagFilter state
introduced, decoupled from tagFilter useMemo
✅ Server-side tag filtering now works in both search and
non-search fetch paths
✅ Java tag lookup optimized from O(n×m) to O(n) using Set
✅ Duplicate interface property removed
✅ Unused imports cleaned up
✅ All 21 tests passing (screenshot attached)

Could you please add the safe-to-test label so CI can run?
Would love to get this reviewed! 🙏

image

Copilot AI review requested due to automatic review settings April 14, 2026 00:41
@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Comment on lines 278 to 286
export type GetTableColumnsParams = {
limit?: number;
offset?: number;
fields?: string;
include?: Include;
sortBy?: 'name' | 'ordinalPosition';
sortOrder?: 'asc' | 'desc';
tags?: string;
};
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.

tags was added to GetTableColumnsParams, but the non-search /tables/{id}/columns and /tables/name/{fqn}/columns endpoints don’t accept a tags query param (they’ll ignore it). This type change can mislead callers into thinking tag filtering works on the list endpoints. Either (a) implement tags on the list endpoints too, or (b) move tags to SearchTableColumnsParams only (or a separate type) so only /columns/search advertises support.

Copilot uses AI. Check for mistakes.
Comment on lines +300 to +324
const tagsParam = buildTagsParam(activeTagFilter);

if (tagsParam) {
const response = await searchTableColumnsByFQN(tableFqn, {
q: '',
limit: pageSize,
offset: offset,
fields: 'tags,customMetrics,extension',
sortBy: sortByParam,
sortOrder: sortOrderParam,
tags: tagsParam,
});
setTableColumns(pruneEmptyChildren(response.data) || []);
handlePagingChange(response.paging);
} else {
const response = await getTableColumnsByFQN(tableFqn, {
limit: pageSize,
offset: offset,
fields: 'tags,customMetrics,extension',
sortBy: sortByParam,
sortOrder: sortOrderParam,
});
setTableColumns(pruneEmptyChildren(response.data) || []);
handlePagingChange(response.paging);
}
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.

When tag filters are active, fetchTableColumns switches to searchTableColumnsByFQN(q: ''). The backend search implementation flattens nested columns before filtering/pagination, whereas the normal /columns endpoint paginates top-level columns and preserves the nested structure. This can regress the schema view for nested columns (loss of expand/collapse, different paging totals/offset semantics, and different “parent matches when child has tag” behavior). Consider adding tags filtering to the regular /columns endpoints (so you can keep using getTableColumnsByFQN with tags) or updating the backend search endpoint to preserve hierarchy when q is empty.

Copilot uses AI. Check for mistakes.
Comment on lines +980 to +986
onChange={(_pagination, filters) => {
setActiveTagFilter({
Classification: (filters[TABLE_COLUMNS_KEYS.TAGS] as string[]) || [],
Glossary: (filters[TABLE_COLUMNS_KEYS.GLOSSARY] as string[]) || [],
});
handlePageChange(1);
}}
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.

The table columns still define onFilter: searchTagInData for both Tags and Glossary columns, so Ant Design will apply client-side filtering in addition to the server-side tags param. This becomes inconsistent when both filters are set: the server returns results using OR semantics across all tag FQNs, but the table will apply AND across the two columns’ filters, and paging totals from the server can become incorrect (pages can appear empty). To make the behavior consistent, either disable client-side filtering for these columns when using server filtering (e.g., remove/neutralize onFilter) or change the backend/UI semantics so both sides apply the same AND/OR logic.

Copilot uses AI. Check for mistakes.
Comment on lines +3239 to +3250
if (tags != null && !tags.trim().isEmpty()) {
Set<String> tagFQNSet = Arrays.stream(tags.split(","))
.map(String::trim)
.filter(s -> !s.isEmpty())
.collect(Collectors.toSet());
matchingColumns = new ArrayList<>(
matchingColumns.stream()
.filter(column -> column.getTags() != null
&& column.getTags().stream()
.anyMatch(tag -> tagFQNSet.contains(
tag.getTagFQN())))
.toList());
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.

searchTableColumnsInternal flattens nested columns and then applies tag filtering only on column.getTags(). This means (1) a parent column with no direct tags but with a tagged child won’t match (UI’s previous recursive filtering matched parents via children), and (2) results returned to the UI are a flat list, losing hierarchy. If the schema table is expected to preserve nesting during tag filtering, consider filtering on the hierarchical structure (recursively) and returning the original tree, or move tag filtering to the non-search columns endpoint which already preserves nesting.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Apr 14, 2026

Code Review ✅ Approved 4 resolved / 4 findings

Adds server-side tag filtering for table columns via ColumnResource API, resolving the infinite render loop, inconsistent parameter passing, inefficient O(n×m) filtering, and pagination reset issues. No remaining issues found.

✅ 4 resolved
Bug: Infinite render loop: tagFilter → fetch → setColumns → tagFilter

📄 openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx:229-236 📄 openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx:280 📄 openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx:323 📄 openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx:350 📄 openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx:362-366
The tagFilter memo (line 229) depends on tableColumns. Both fetchTableColumns and searchTableColumns callbacks depend on tagFilter (lines 323, 280). These callbacks call setTableColumns, which changes tableColumns, which recomputes tagFilter (producing a new object reference from groupBy), which recreates the callbacks, which triggers the useEffect hooks (lines 352-366, 346-350) that call those callbacks again — creating an infinite fetch loop.

The root cause is that tagFilter is derived from the fetched columns, but is also used as a parameter to fetch columns. This circular dependency means every fetch triggers another fetch.

The fundamental design issue: tagFilter as computed here represents the set of tags visible on the current page (for populating the filter dropdown). It should NOT be used as the server-side filter parameter. The user's active filter selection (a separate piece of state) should be sent to the server. The available filter options (derived from columns) and the active filter selection are two different things.

To fix this, you need a separate activeTagFilter state that captures what the user has actually selected in the filter UI, and pass only that to the API. The tagFilter memo (which populates filter dropdown options) should remain derived from tableColumns but must NOT be in the dependency arrays of the fetch callbacks.

Quality: Inconsistent tags param passing between fetch and search

📄 openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx:264 📄 openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx:307
In searchTableColumns (line 264), tags: tagsParam is always passed (even when undefined). In fetchTableColumns (line 307), it uses conditional spread ...(tagsParam ? { tags: tagsParam } : {}). While functionally equivalent (undefined values are typically stripped by axios), the inconsistency is confusing. Pick one pattern.

Performance: O(n×m) tag filtering with stream-in-stream in Java

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java:3239-3253
The tag filtering in searchTableColumnsInternal (lines 3239-3254) iterates over all columns, then for each column iterates over its tags, and for each tag iterates over tagFQNs. For tables with many columns and tags, this is O(columns × tags × filterTags). Converting tagFQNs to a Set<String> would make the inner lookup O(1) instead of O(m).

Bug: Pagination not reset to page 1 when tag filter changes

📄 openmetadata-ui/src/main/resources/ui/src/components/Database/SchemaTable/SchemaTable.component.tsx:980-985
When the user applies a tag filter while on page 2+, activeTagFilter changes and triggers fetchTableColumns(currentPage, ...) with the stale page number. The filtered result set is likely smaller, so the user may see empty results or miss the first page of filtered data.

Sort changes already reset pagination via handlePageChange(1) (see handleSortMenuClick and handleColumnHeaderSortToggle), but the onChange handler for tag filters does not.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Column tag filter works only per page instead of across entire table

2 participants