fix(aws): apply column comments when syncing to Glue

[rangareddy]

Describe the issue this Pull Request addresses

Closes #19316.

AWSGlueCatalogSyncClient.updateTableComments has applied no column or partition column comments since the
AWS SDK v2 upgrade (#9347). Its helper built a Column carrying the comment and threw the result away:

private void setComments(List<Column> columns, Map<String, Option<String>> commentsMap) {
  columns.forEach(column -> {
    String comment = commentsMap.getOrDefault(column.name(), Option.empty()).orElse(null);
    Column.builder().comment(comment).build();   // result dropped, column unchanged
  });
}

Before the upgrade this called column.setComment(...) on the mutable v1 model, which worked. SDK v2 model
classes are immutable, so nothing was applied: updateTableComments never detected a change, always
returned false, and with hoodie.datasource.hive_sync.sync_comment=true no comment ever reached Glue.

Found while reviewing #19289, which fixed the equivalent Hive metastore paths.

Summary and Changelog

• setComments becomes withComments, which returns a rebuilt list instead of mutating in place.

• The storage descriptor is rebuilt too. Rebuilding only the column list is not enough and is the part
  worth reviewing: StorageDescriptor is immutable as well, and the UpdateTableRequest was sending the
  original descriptor. Editing a copy of storageDescriptor.columns() — which is what the issue text
  originally suggested — would still have shipped columns with no comments. The request now sends the
  descriptor rebuilt from the updated columns.

• A column the storage schema says nothing about is left untouched rather than cleared. The pre-SDK-v2
  code cleared it, but since that code has been a no-op for three years nothing depends on it, and clearing
  is the riskier reading: getStorageFieldSchemas keeps the Avro schema's case while a catalog may hold
  column names lowercased, so a name that failed to match would silently wipe a user's comment. Columns the
  schema does know are still authoritative — a known column with no doc has its comment cleared. This
  matches HMSDDLExecutor.applyFieldComments, added for the Hive side in fix(hive-sync): sync column and partition column comments to HMS #19289, so the two catalogs now
  agree.

• Change detection now uses the table already fetched. It compared a freshly fetched table against local
  objects it had not modified — trivially equal, and two extra Glue GetTable calls per sync. It now
  compares the fetched table against the rebuilt values, so one GetTable call does the job.

Verification

withComments is @VisibleForTesting and covered by four new tests in TestAWSGlueSyncClient:

test	what it pins
testWithCommentsAppliesTheStorageComment	a missing comment is applied, a stale one replaced, and the input list is not mutated
testWithCommentsClearsTheCommentOfAKnownColumnWithoutADoc	the schema is authoritative for columns it knows
testWithCommentsLeavesColumnsTheStorageSchemaDoesNotKnowAlone	an unknown column's comment is preserved
testRebuildingColumnsRequiresRebuildingTheStorageDescriptor	storageDescriptor.columns() is unmodifiable, and a descriptor rebuilt with new columns is a different object — the trap the original bug fell into

Restoring the build-and-drop behaviour inside withComments turns three of them red, so they are not
passing vacuously:

[ERROR] testWithCommentsAppliesTheStorageComment
  AssertionFailedError: a missing comment should be applied ==> expected: <person's name> but was: <null>
[ERROR] testWithCommentsClearsTheCommentOfAKnownColumnWithoutADoc
  AssertionFailedError: ... ==> expected: <null> but was: <old comment>
[ERROR] testRebuildingColumnsRequiresRebuildingTheStorageDescriptor
  AssertionFailedError: the rebuilt descriptor carries the comment ==> expected: <person's name> but was: <null>

Whole hudi-aws module: Tests run: 99, Failures: 0, Errors: 0, Skipped: 16 (skips pre-existing).
checkstyle:check and apache-rat:check clean.

Coverage gap now closed. codecov reported 58.33% patch coverage with 5 uncovered lines, all inside
updateTableComments — the method this PR is about. The tests drove withComments directly because
updateTableComments was unreachable from this module: it calls getTableDoc(), which resolves the table
schema, and the fixture had none.

The cause was that GlueTestUtil wrote its commit as JSON into .hoodie, while this is a table-version-8+
table whose active timeline lives under .hoodie/timeline and is read through CommitMetadataSerDe. So the
instant was not on the timeline at all (getActiveTimeline() returned []), and putting it in the right place
by hand still failed, because CommitMetadataSerDe exposes only deserialize.

The fixture now writes the commit through HoodieTestTable, as the rest of the repo does, which needs two
test-jar dependencies: hudi-hadoop-common for HoodieTestTable and hudi-common for the FileCreateUtils
it delegates to. Both are declared exactly as the sibling hudi-gcp and hudi-azure modules declare them —
27 modules already depend on the hudi-hadoop-common test-jar — and there is no dependency cycle.
GlueTestUtil's hand-rolled createMetaFile is dead as a result and is removed.

Two tests now drive updateTableComments end to end:

test	what it pins
testUpdateTableCommentsAppliesThemToColumnsAndPartitionKeys	the captured UpdateTableRequest carries the comments on both the storage descriptor's columns and the partition keys, and the method reports a change
testUpdateTableCommentsIsANoOpWhenNothingChanges	comments already matching the storage schema produce no updateTable call at all

The end-to-end test earns its keep on the half the unit tests could not reach. Rebuilding the column list
but sending the original StorageDescriptor — the mistake the issue's own suggested fix would have made —
fails only this new test and nothing else in the suite:

testUpdateTableCommentsAppliesThemToColumnsAndPartitionKeys:291
  the rebuilt storage descriptor must be the one sent, carrying the column comment
  ==> expected: <person's name> but was: <null>

Whole hudi-aws module: Tests run: 101, Failures: 0, Errors: 0, Skipped: 16 (skips pre-existing).
checkstyle:check and apache-rat:check clean.

Impact

With hoodie.datasource.hive_sync.sync_comment=true, Glue column and partition column comments start being
applied on the update path, which is the documented behaviour and what worked before the SDK v2 upgrade.
Users who already have comments in Glue keep them: only columns the storage schema knows are touched.

Two extra GetTable calls per comment sync are removed. No API, config or table format change.

Risk Level

low — one helper rewritten and its result actually used, in a path that currently does nothing at all.
The semantics of the "unknown column" case are deliberately narrower than the pre-SDK-v2 code; that is
argued above rather than hidden.

Documentation Update

none — no new config, and this restores documented behaviour rather than changing it.

Contributor's checklist

• Read through contributor's guide
• Enough context is provided in the sections above
• Adequate tests were added if applicable
• CI passes on my PR

[hudi-agent]

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! This PR fixes AWSGlueCatalogSyncClient column/partition comment syncing, which had been a silent no-op since the AWS SDK v2 upgrade — SDK v2 model classes are immutable, so the old setComments built a Column and discarded it. The fix rebuilds the columns and, importantly, the enclosing StorageDescriptor so the UpdateTableRequest actually carries the comments, and reworks change detection to compare the original against the rebuilt descriptor. The immutability handling, unchanged-column short-circuit, and unknown-column preservation all look correct and are well covered by the new tests. No correctness issues found. A few style/readability suggestions in the inline comments. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. Code looks clean overall — the withComments rename, immutability fix, and test coverage are all well-done; one minor doc phrasing note below.

cc @yihua

[codecov-commenter]

Codecov Report

❌ Patch coverage is 58.33333% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.97%. Comparing base (637996c) to head (ac27902).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
...apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java	58.33%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master   #19488   +/-   ##
=========================================
  Coverage     76.97%   76.97%           
- Complexity    33855    33862    +7     
=========================================
  Files          2575     2575           
  Lines        143378   143378           
  Branches      17573    17573           
=========================================
+ Hits         110362   110368    +6     
+ Misses        24755    24751    -4     
+ Partials       8261     8259    -2     

Components	Coverage Δ
hudi-common	82.27% <ø> (+<0.01%)	⬆️
hudi-client	81.83% <ø> (ø)
hudi-flink	83.97% <ø> (+<0.01%)	⬆️
hudi-spark-datasource	75.10% <ø> (ø)
hudi-utilities	73.67% <ø> (+0.01%)	⬆️
hudi-cli	15.32% <ø> (ø)
hudi-hadoop	63.49% <ø> (ø)
hudi-sync	70.92% <ø> (+0.05%)	⬆️
hudi-io	79.60% <ø> (ø)
hudi-timeline-service	83.44% <ø> (-0.79%)	⬇️
hudi-cloud	64.32% <58.33%> (+0.32%)	⬆️
hudi-kafka-connect	53.20% <ø> (ø)

Flag	Coverage Δ
common-and-other-modules	49.54% <58.33%> (+<0.01%)	⬆️
flink-integration-tests	48.80% <0.00%> (+<0.01%)	⬆️
integration-tests	13.58% <0.00%> (-0.01%)	⬇️
spark-client-hadoop-common	48.67% <ø> (+<0.01%)	⬆️
spark-java-tests	51.32% <0.00%> (-0.02%)	⬇️
spark-scala-tests	47.40% <0.00%> (+<0.01%)	⬆️
utilities	36.58% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...apache/hudi/aws/sync/AWSGlueCatalogSyncClient.java	51.96% <58.33%> (+1.06%)	⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[hudi-agent]

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! This PR fixes AWS Glue column-comment syncing that had silently no-op'd since the SDK v2 upgrade, by rebuilding the immutable Column/StorageDescriptor objects instead of discarding the built result. The withComments logic (preserving unknown columns, clearing known-but-undoc'd ones) and the switch to comparing the rebuilt descriptor against the already-fetched one both trace out correctly, and the tests cover the key cases. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.

cc @yihua

[hudi-bot]

CI report:

• 1b6c3e9 Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

• @hudi-bot run azure re-run the last Azure build

[hudi-agent]

⚠️ 🤖 This review was generated by an AI agent and may contain mistakes. Please verify any suggestions before applying.

Thanks for working on this! The PR fixes AWSGlueCatalogSyncClient so column and partition-column comments are actually applied when syncing to Glue by rebuilding the immutable SDK v2 Column list and StorageDescriptor instead of discarding the rebuilt column, and correcting the always-false no-op comparison. No issues flagged from this automated pass, a Hudi committer or PMC member can take it from here for a final review.

cc @yihua