yihua commented on code in PR #19427:
URL: https://github.com/apache/hudi/pull/19427#discussion_r3688069645
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncTool.java:
##########
@@ -211,7 +211,16 @@ protected void doSync() {
syncHoodieTable(snapshotTableName, true, false);
// sync origin table for MOR
if (config.getBoolean(META_SYNC_SNAPSHOT_WITH_TABLE_NAME)) {
- syncHoodieTable(tableName, true, false);
+ if
(config.getBoolean(HIVE_SKIP_RO_SUFFIX_FOR_READ_OPTIMIZED_TABLE)) {
+ // skip_ro_suffix explicitly claims the bare table name for
the RO view;
+ // an implicit sync_snapshot_with_table_name default must not
repurpose it as RT.
+ log.warn("{}=true claims the base table name '{}' for the
read-optimized view; "
Review Comment:
Done, reworded to "bare table name".
##########
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncTool.java:
##########
@@ -211,7 +211,16 @@ protected void doSync() {
syncHoodieTable(snapshotTableName, true, false);
// sync origin table for MOR
if (config.getBoolean(META_SYNC_SNAPSHOT_WITH_TABLE_NAME)) {
- syncHoodieTable(tableName, true, false);
+ if
(config.getBoolean(HIVE_SKIP_RO_SUFFIX_FOR_READ_OPTIMIZED_TABLE)) {
+ // skip_ro_suffix explicitly claims the bare table name for
the RO view;
+ // an implicit sync_snapshot_with_table_name default must not
repurpose it as RT.
Review Comment:
Done, removed.
##########
hudi-sync/hudi-hive-sync/src/test/java/org/apache/hudi/hive/TestHiveSyncTool.java:
##########
@@ -1557,6 +1559,43 @@ public void testSyncMergeOnReadWithStrategy(String
syncMode, HoodieSyncTableStra
}
}
+ @Test
+ void testSkipRoSuffixTakesPrecedenceOverSnapshotWithTableName() throws
Exception {
+ // skip_ro_suffix explicitly claims the bare table name for the RO view;
the now-default-true
+ // sync_snapshot_with_table_name must not be allowed to flip it to RT.
+ hiveSyncProps.setProperty(HIVE_SYNC_TABLE_STRATEGY.key(),
HoodieSyncTableStrategy.ALL.name());
+
hiveSyncProps.setProperty(HIVE_SKIP_RO_SUFFIX_FOR_READ_OPTIMIZED_TABLE.key(),
"true");
+ hiveSyncProps.setProperty(META_SYNC_SNAPSHOT_WITH_TABLE_NAME.key(),
"true");
+ hiveSyncProps.setProperty(HIVE_SYNC_AS_DATA_SOURCE_TABLE.key(), "true");
+
+ String instantTime = "100";
+ String deltaCommitTime = "101";
+ HiveTestUtil.createMORTable(instantTime, deltaCommitTime, 5, true, true);
+
+ reInitHiveSyncClient();
+ reSyncHiveTable();
Review Comment:
Verified: reverted the production fix and ran the test — it fails as
expected on the first commit alone (`isAlreadySynced` short-circuits the
redundant sync before it can flip anything), and only fails with the right
symptom once a second commit + resync round is added, matching what you found
locally. Added the second round (`HiveTestUtil.addMORPartitions` + another
`reSyncHiveTable()`), confirmed it fails without the fix (bare table flips to
`HoodieParquetRealtimeInputFormat`) and passes with it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
