wombatu-kun commented on code in PR #19288:
URL: https://github.com/apache/hudi/pull/19288#discussion_r3663203416
##########
hudi-trino/src/main/java/io/trino/plugin/hudi/HudiUtil.java:
##########
@@ -471,12 +624,94 @@ public static List<HiveColumnHandle>
getMergeRequiredColumnHandles(
}
}
- if (requiredColumnNames.isEmpty()) {
- return Collections.emptyList();
- }
return buildColumnHandles(table, typeManager, requiredColumnNames,
timestampPrecision);
}
+ /**
+ * The merge-mandatory column names that do not depend on a custom merger,
mirroring the non-CUSTOM
+ * branch of {@code
FileGroupReaderSchemaHandler.getMandatoryFieldsForMerging}. The delete-marker
and
+ * operation fields are added unconditionally: {@code buildColumnHandles}
keeps only metastore data
+ * columns, and the merge read path recovers any name the metastore does
not carry from the resolved
+ * table schema ({@link #appendMissingMergeRequiredColumns}). For those
two fields that mirrors the
+ * file-group reader's own schema gate, so tables whose schema has neither
read nothing extra; the
+ * ordering, record-key and delete-key names the reader adds without a
schema check, but they are
+ * table columns whenever the table config is coherent.
+ */
+ @VisibleForTesting
+ static LinkedHashSet<String> mergeRequiredColumnNames(HoodieTableConfig
tableConfig, RecordMergeMode recordMergeMode)
+ {
+ LinkedHashSet<String> requiredColumnNames = new LinkedHashSet<>();
+ if (recordMergeMode != null && recordMergeMode !=
RecordMergeMode.COMMIT_TIME_ORDERING) {
+ requiredColumnNames.addAll(tableConfig.getOrderingFields());
+ }
+ // Without populated meta fields the file-group reader keys records on
the record-key data columns
+ if (!tableConfig.populateMetaFields()) {
+ tableConfig.getRecordKeyFields().ifPresent(fields ->
requiredColumnNames.addAll(Arrays.asList(fields)));
+ }
+ // Delete markers and the operation field decide record deletion at
merge time
+ requiredColumnNames.add(HOODIE_IS_DELETED_FIELD);
+ requiredColumnNames.add(OPERATION_METADATA_FIELD);
+ String deleteKey = tableConfig.getProps().getProperty(DELETE_KEY);
+ String deleteMarker =
tableConfig.getProps().getProperty(DELETE_MARKER);
+ // DeleteContext only honors a custom delete key when the marker value
is also set
+ if (!StringUtils.isNullOrEmpty(deleteKey) &&
!StringUtils.isNullOrEmpty(deleteMarker)) {
+ requiredColumnNames.add(deleteKey);
+ }
+ return requiredColumnNames;
+ }
+
+ /**
+ * Returns {@code projection} extended with handles for the merge-required
column names it does not
+ * already carry (matched case-insensitively) and the metastore could not
resolve, typed from
+ * {@code dataSchema} via {@link #toColumnHandle}: the
table-config-derived names
+ * ({@link #mergeRequiredColumnNames}) plus, under a CUSTOM merge mode,
the mandatory fields declared
+ * by the same resolved merger the file-group reader will use. Names
absent from the schema as well are
+ * dropped -- for {@code _hoodie_is_deleted} and {@code _hoodie_operation}
that mirrors the file-group
+ * reader's own schema gate; the rest are table columns whenever the table
config is coherent. A
+ * metastore that omits a field the table schema carries -- hive sync with
+ * {@code omit_metadata_fields=true} drops {@code _hoodie_operation},
hand-written external DDL can drop
+ * any data column -- must not starve the base-file read of it; the
+ * {@code HudiTrinoReaderContext.getFileRecordIterator} guard would
otherwise fail the read. Called on
+ * the merge read path only, where the table schema is already resolved,
so the recovery costs no I/O.
+ */
+ public static List<HiveColumnHandle> appendMissingMergeRequiredColumns(
+ HoodieSchema dataSchema,
+ List<HiveColumnHandle> projection,
+ HoodieTableConfig tableConfig,
+ TypedProperties readerProps)
+ {
+ Pair<RecordMergeMode, String> mergeModeAndStrategyId =
resolveMergeModeAndStrategyId(tableConfig);
+ LinkedHashSet<String> requiredNames =
mergeRequiredColumnNames(tableConfig, mergeModeAndStrategyId.getLeft());
+ // A CUSTOM merger can declare mandatory fields of its own
(getMandatoryFieldsForMerging); ask the
+ // same resolved merger the file-group reader will use, against the
same resolved schema, so its
+ // declarations are recovered too. Resolution is classloading only --
no I/O.
+ if (mergeModeAndStrategyId.getLeft() == RecordMergeMode.CUSTOM &&
!StringUtils.isNullOrEmpty(mergeModeAndStrategyId.getRight())) {
+ String mergeImplClasses =
readerProps.getString(RECORD_MERGE_IMPL_CLASSES_WRITE_CONFIG_KEY,
+
readerProps.getString(RECORD_MERGE_IMPL_CLASSES_DEPRECATED_WRITE_CONFIG_KEY,
""));
+ Option<HoodieRecordMerger> merger =
HoodieRecordUtils.createValidRecordMerger(EngineType.JAVA, mergeImplClasses,
mergeModeAndStrategyId.getRight());
+ if (merger.isPresent()) {
Review Comment:
The empty-strategy-id and unresolvable-merger guards this branch adds have
no test, while the same two cases are pinned for
`usesNonProjectionCompatibleMerger` in `TestHudiUtilColumnHandles`. Worth
mirroring those two assertions onto `appendMissingMergeRequiredColumns`, so
removing either guard fails a test instead of turning the `HUDI_BAD_DATA` path
back into an NPE.
--
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]
