hudi-agent commented on code in PR #19304:
URL: https://github.com/apache/hudi/pull/19304#discussion_r3633296272
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -1544,6 +1548,36 @@ protected boolean loadActiveTimelineOnTableInit() {
return true;
}
+ private void handleComplexKeygenEncoding(HoodieTableMetaClient metaClient) {
+ HoodieTableConfig tableConfig = metaClient.getTableConfig();
+ if
(!tableConfig.getTableVersion().lesserThan(org.apache.hudi.common.table.HoodieTableVersion.NINE))
{
+ return; // table version 9+ uses fixed encoding; no deduction needed
Review Comment:
🤖 nit: `HoodieTableVersion` is already imported (used as
`HoodieTableVersion.NINE` without a qualifier just a few dozen lines below),
and `Option` is used by simple name throughout the class — so the two
fully-qualified names in this method
(`org.apache.hudi.common.table.HoodieTableVersion.NINE` here and
`org.apache.hudi.common.util.Option<Boolean>` a few lines down) look like
copy-paste leftovers. Could you drop the package prefixes for consistency?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java:
##########
@@ -385,4 +408,91 @@ public static boolean
mayUseNewEncodingForComplexKeyGen(HoodieTableConfig tableC
return tableConfig.getTableVersion().lesserThan(HoodieTableVersion.NINE)
&& isComplexKeyGeneratorWithSingleRecordKeyField(tableConfig);
}
+
+ public static StoragePath getComplexKeyEncodingFilePath(StoragePath
basePath) {
+ return new StoragePath(basePath, AUXILIARYFOLDER_NAME + "/" +
COMPLEX_KEY_ENCODING_FILE_NAME);
+ }
+
+ public static Option<Boolean>
readComplexKeyEncodingFromAuxFile(HoodieStorage storage, StoragePath basePath) {
+ StoragePath encodingFilePath = getComplexKeyEncodingFilePath(basePath);
+ try {
+ if (storage.exists(encodingFilePath)) {
+ Properties props = new Properties();
+ try (InputStream inputStream = storage.open(encodingFilePath)) {
+ props.load(inputStream);
+ }
+ String value = props.getProperty(COMPLEX_KEYGEN_NEW_ENCODING.key());
+ if (value != null) {
+ return Option.of(Boolean.parseBoolean(value));
+ }
+ }
+ } catch (IOException e) {
+ LOG.warn("Failed to read complex key encoding from aux file: {}",
encodingFilePath, e);
+ }
+ return Option.empty();
+ }
+
+ public static void writeComplexKeyEncodingToAuxFile(HoodieStorage storage,
StoragePath basePath, boolean useNewEncoding) {
+ StoragePath encodingFilePath = getComplexKeyEncodingFilePath(basePath);
+ try {
+ Properties props = new Properties();
+ props.setProperty(COMPLEX_KEYGEN_NEW_ENCODING.key(),
String.valueOf(useNewEncoding));
+ try (OutputStream outputStream = storage.create(encodingFilePath, true))
{
+ props.store(outputStream, "Complex key generator encoding format");
+ }
+ LOG.info("Wrote complex key encoding to aux file: {}", useNewEncoding);
+ } catch (IOException e) {
+ throw new HoodieKeyException("Failed to write complex key encoding file
to " + encodingFilePath, e);
+ }
+ }
+
+ public static final boolean DEFAULT_NEW_ENCODING_FOR_NEW_TABLE = true;
+
+ public static boolean deduceComplexKeyEncodingFromData(HoodieTableMetaClient
metaClient, String recordKeyFieldName) {
+ HoodieTimeline completedTimeline =
metaClient.getActiveTimeline().getCommitsTimeline().filterCompletedInstants();
+ if (completedTimeline.empty()) {
+ LOG.info("No completed commits found in table {}; defaulting complex key
encoding to useNewEncoding={} (new/empty table).",
+ metaClient.getBasePath(), DEFAULT_NEW_ENCODING_FOR_NEW_TABLE);
+ return DEFAULT_NEW_ENCODING_FOR_NEW_TABLE;
+ }
+
+ try {
+ HoodieStorage storage = metaClient.getStorage();
+ FileFormatUtils fileFormatUtils = HoodieIOFactory.getIOFactory(storage)
+ .getFileFormatUtils(HoodieFileFormat.PARQUET);
+
+ List<HoodieInstant> instants =
completedTimeline.getReverseOrderedInstants().collect(Collectors.toList());
+ for (HoodieInstant instant : instants) {
+ HoodieCommitMetadata commitMetadata =
TimelineUtils.getCommitMetadata(instant, completedTimeline);
+ for (HoodieWriteStat writeStat : commitMetadata.getWriteStats()) {
+ String filePath = writeStat.getPath();
+ if (filePath == null || filePath.isEmpty() ||
!filePath.endsWith(".parquet")) {
+ continue;
Review Comment:
🤖 nit: `HoodieFileFormat.PARQUET` is already in scope (used on line 462), so
it might be worth deriving the extension from the enum rather than hard-coding
`".parquet"` here — something like
`HoodieFileFormat.PARQUET.getFileExtension()`. That way the filter stays in
sync with the format constant automatically.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/keygen/KeyGenUtils.java:
##########
@@ -385,4 +408,91 @@ public static boolean
mayUseNewEncodingForComplexKeyGen(HoodieTableConfig tableC
return tableConfig.getTableVersion().lesserThan(HoodieTableVersion.NINE)
&& isComplexKeyGeneratorWithSingleRecordKeyField(tableConfig);
}
+
+ public static StoragePath getComplexKeyEncodingFilePath(StoragePath
basePath) {
+ return new StoragePath(basePath, AUXILIARYFOLDER_NAME + "/" +
COMPLEX_KEY_ENCODING_FILE_NAME);
+ }
+
+ public static Option<Boolean>
readComplexKeyEncodingFromAuxFile(HoodieStorage storage, StoragePath basePath) {
+ StoragePath encodingFilePath = getComplexKeyEncodingFilePath(basePath);
+ try {
+ if (storage.exists(encodingFilePath)) {
+ Properties props = new Properties();
+ try (InputStream inputStream = storage.open(encodingFilePath)) {
+ props.load(inputStream);
+ }
+ String value = props.getProperty(COMPLEX_KEYGEN_NEW_ENCODING.key());
+ if (value != null) {
+ return Option.of(Boolean.parseBoolean(value));
+ }
+ }
+ } catch (IOException e) {
+ LOG.warn("Failed to read complex key encoding from aux file: {}",
encodingFilePath, e);
+ }
+ return Option.empty();
+ }
+
+ public static void writeComplexKeyEncodingToAuxFile(HoodieStorage storage,
StoragePath basePath, boolean useNewEncoding) {
+ StoragePath encodingFilePath = getComplexKeyEncodingFilePath(basePath);
+ try {
+ Properties props = new Properties();
+ props.setProperty(COMPLEX_KEYGEN_NEW_ENCODING.key(),
String.valueOf(useNewEncoding));
+ try (OutputStream outputStream = storage.create(encodingFilePath, true))
{
+ props.store(outputStream, "Complex key generator encoding format");
+ }
+ LOG.info("Wrote complex key encoding to aux file: {}", useNewEncoding);
+ } catch (IOException e) {
+ throw new HoodieKeyException("Failed to write complex key encoding file
to " + encodingFilePath, e);
+ }
+ }
+
+ public static final boolean DEFAULT_NEW_ENCODING_FOR_NEW_TABLE = true;
+
+ public static boolean deduceComplexKeyEncodingFromData(HoodieTableMetaClient
metaClient, String recordKeyFieldName) {
Review Comment:
🤖 nit: `DEFAULT_NEW_ENCODING_FOR_NEW_TABLE` is a `public static final`
constant but it's declared between two method bodies — could you move it up to
the top of the class alongside `COMPLEX_KEY_ENCODING_FILE_NAME` and the other
constants? Java convention puts all static fields at the top, and readers
typically scan the constant block first when looking for defaults.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
