Conversation
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
CI is blocked by the repository label gate. A maintainer needs to add the \safe to test\ label before the required checks can run past the label verification step. |
There was a problem hiding this comment.
Pull request overview
Fixes missing column-level tag hydration for table reads when only fields=columns is requested, while keeping table-level tags gated behind fields=tags for bulk reads (per #27519).
Changes:
- Hydrate column tags whenever
columnsare requested in single-table reads (setFields). - Update bulk hydration to fetch column tags when
columnsare requested, but only fetch table tags whentagsare requested. - Add regression tests covering single-table and bulk hydration paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TableRepository.java | Ensures column tag hydration runs when columns are requested; avoids bulk table-tag fetch unless tags is requested. |
| openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/TableRepositoryColumnTagsTest.java | Adds regression coverage for column tag hydration in both single-entity and bulk paths. |
| @Test | ||
| void setFieldsInBulk_hydratesColumnTagsWhenOnlyColumnsAreRequested() { | ||
| Table table = tableWithColumn("service.database.schema.users", "email"); | ||
| TagLabel piiTag = | ||
| new TagLabel() | ||
| .withTagFQN("PII.Sensitive") | ||
| .withSource(TagSource.CLASSIFICATION) | ||
| .withLabelType(LabelType.MANUAL) | ||
| .withState(State.CONFIRMED); | ||
| String columnFqn = table.getColumns().getFirst().getFullyQualifiedName(); | ||
|
|
||
| when(tagUsageDAO.getTagsInternalBatch(anyList())) | ||
| .thenReturn(List.of(tagUsage(columnFqn, piiTag))); | ||
|
|
||
| repository.setFieldsInBulk( | ||
| new Fields(repository.getAllowedFields(), "columns"), List.of(table)); | ||
|
|
||
| assertNull(table.getTags()); | ||
| assertColumnTags(table, piiTag); | ||
| } |
There was a problem hiding this comment.
In setFieldsInBulk_hydratesColumnTagsWhenOnlyColumnsAreRequested, assertNull(table.getTags()) doesn’t prove bulk table-level tags were not fetched, since clearFieldsInternal will null out table.tags whenever fields doesn’t include tags (even if they were loaded). To fully regression-test the “keep top-level tags gated behind fields=tags” behavior, add a Mockito verification that the tag DAO is only queried for the column FQN(s) (and not also for the table FQN) when Fields is just columns.
There was a problem hiding this comment.
Fixed in 5a6730c by stubbing and verifying getTagsInternalBatch with only the column FQN, then asserting no more tag DAO interactions. The bulk test now fails if this path also fetches table-level tags.
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Code Review ✅ ApprovedFixes incorrect hydration of table column tags to ensure proper UI rendering. No issues were found. OptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
| package org.openmetadata.service.jdbi3; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; |
There was a problem hiding this comment.
New Java test file is missing the standard Apache 2.0 license header comment block that other files in this module include (e.g., BoundedListFilterTest.java:1-12). This is typically enforced by build/format checks and should be added above the package declaration.
Summary
fields=tagsin bulk table reads.Fixes #27519
Validation
git diff --cached --checkgh pr checks 27522 --repo open-metadata/OpenMetadataas the current green Java-service CI baselinemvn -pl openmetadata-service -Dtest=TableRepositoryColumnTagsTest testlocally becausemvnandjavaare not installed in this environment.