hudi-agent commented on code in PR #18384:
URL: https://github.com/apache/hudi/pull/18384#discussion_r3271036787
##########
hudi-client/hudi-flink-client/src/main/java/org/apache/hudi/io/storage/row/HoodieRowDataCreateHandle.java:
##########
@@ -143,13 +146,28 @@ public void write(String recordKey, String partitionPath,
RowData record) throws
String commitInstant;
RowData rowData;
if (!skipMetadataWrite) {
- seqId = preserveHoodieMetadata
- ?
record.getString(HoodieRecord.COMMIT_SEQNO_METADATA_FIELD_ORD).toString()
- : HoodieRecord.generateSequenceId(instantTime, taskPartitionId,
SEQGEN.getAndIncrement());
- commitInstant = preserveHoodieMetadata
- ?
record.getString(HoodieRecord.COMMIT_TIME_METADATA_FIELD_ORD).toString()
- : instantTime;
- rowData = HoodieRowDataCreation.create(commitInstant, seqId,
recordKey, partitionPath, path.getName(),
+ if (hoodieMetaFieldFlags.isCommitTimePopulated()) {
Review Comment:
🤖 nit: this nested if/else for commit_time / commit_seqno / record_key /
partition_path / file_name reads as 20+ lines of branching. Since
`HoodieMetaFieldFlags` already has `getCommitTime(String)` /
`getRecordKey(String)` / etc. helpers that return value-or-null, could you
collapse most of these to one-liners (and just keep the
`preserveHoodieMetadata`/`isNullAt` branch explicit)? Would make the write path
much easier to scan.
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -971,7 +972,12 @@ private static <T> HoodieData<HoodieRecord>
readRecordKeysFromFileSliceSnapshot(
final String fileId = fileSlice.getFileId();
HoodieReaderContext<T> readerContext = readerContextFactory.getContext();
HoodieSchema dataSchema =
HoodieSchemaCache.intern(HoodieSchemaUtils.addMetadataFields(HoodieSchema.parse(dataWriteConfig.getWriteSchema()),
dataWriteConfig.allowOperationMetadataField()));
- HoodieSchema requestedSchema =
metaClient.getTableConfig().populateMetaFields() ? getRecordKeySchema()
+ // Project _hoodie_record_key from the file only when it is actually
populated on
+ // disk. With selective exclusion (META_FIELDS_EXCLUDE_LIST), the column
is null and
+ // we must project the configured source record-key fields instead so
the record-index
+ // is rebuilt from the user data columns.
Review Comment:
🤖 nit: most other call sites in this PR go through the
`metaClient.getTableConfig().getHoodieMetaFieldFlags()` convenience method (and
its own javadoc calls the table config the "source of truth"). Could you switch
this site to the same form for consistency?
`metaClient.getTableConfig().getHoodieMetaFieldFlags().isRecordKeyPopulated()`.
<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]
