hudi-agent commented on code in PR #18837:
URL: https://github.com/apache/hudi/pull/18837#discussion_r3350626144
##########
hudi-trino-plugin/src/main/java/io/trino/plugin/hudi/HudiUtil.java:
##########
@@ -397,4 +412,52 @@ public static Schema
getLatestTableSchema(HoodieTableMetaClient metaClient, Stri
throw new TrinoException(HUDI_FILESYSTEM_ERROR, e);
}
}
+
+ public static List<HiveColumnHandle> getOrderingColumnHandles(Table table,
TypeManager typeManager, Lazy<HoodieTableMetaClient> lazyMetaClient,
HiveTimestampPrecision timestampPrecision)
+ {
+ RecordMergeMode recordMergeMode =
lazyMetaClient.get().getTableConfig().getRecordMergeMode();
+ if (recordMergeMode == null || recordMergeMode ==
RecordMergeMode.COMMIT_TIME_ORDERING) {
+ // if commit time ordering is enabled, return empty list
+ return Collections.emptyList();
Review Comment:
🤖 nit: the comment says "if commit time ordering is enabled" but the guard
also fires when `recordMergeMode` is `null`. Something like `// no merge mode
configured or commit-time ordering — no ordering columns needed` would cover
both cases.
<sub><i>- AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-trino-plugin/src/main/java/io/trino/plugin/hudi/reader/HudiTrinoReaderContext.java:
##########
@@ -74,9 +79,26 @@ public ClosableIterator<IndexedRecord> getFileRecordIterator(
StoragePath storagePath,
long start,
long length,
- Schema dataSchema,
- Schema requiredSchema,
+ HoodieSchema dataSchema,
+ HoodieSchema requiredSchema,
HoodieStorage storage)
+ {
+ return createRecordIterator();
+ }
+
+ @Override
+ public ClosableIterator<IndexedRecord> getFileRecordIterator(
+ StoragePathInfo storagePathInfo,
Review Comment:
🤖 nit: both `getFileRecordIterator` overloads silently ignore every
parameter and delegate to `createRecordIterator()`. Without a short comment
it's easy to wonder whether this is an incomplete stub. Could you add a line
like `// Parameters are unused: the Trino page source already holds the open
file; createRecordIterator() wraps it.` — similar to what
`mergeBootstrapReaders` does?
<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]
