wombatu-kun commented on code in PR #19288:
URL: https://github.com/apache/hudi/pull/19288#discussion_r3662221275
##########
hudi-trino/src/main/java/io/trino/plugin/hudi/HudiUtil.java:
##########
@@ -471,12 +602,38 @@ 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}
drops any name that is not a
+ * data column of the table, so tables without them read nothing extra.
+ */
+ @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);
Review Comment:
A CUSTOM merger's own mandatory fields are still resolved metastore-only, so
a merger-declared column the metastore lacks (`MaxRankRecordMerger`'s
`merge_rank`) still starves the base read and trips the `getFileRecordIterator`
guard. The call site already holds `readerProps` and the resolved `dataSchema`,
so asking the merger there too would close it with no extra I/O - or is that
out of scope here?
##########
hudi-trino/src/main/java/io/trino/plugin/hudi/util/PrefilledColumnValues.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.trino.plugin.hudi.util;
+
+import com.google.common.collect.ImmutableMap;
+import io.trino.plugin.hive.HiveColumnHandle;
+import io.trino.plugin.hive.HivePartitionKey;
+import io.trino.plugin.hudi.HudiSplit;
+import io.trino.plugin.hudi.file.HudiFile;
+import io.trino.spi.block.Block;
+import io.trino.spi.block.BlockBuilder;
+import io.trino.spi.block.RunLengthEncodedBlock;
+
+import java.util.List;
+import java.util.Map;
+import java.util.OptionalInt;
+
+import static io.trino.metastore.Partitions.makePartName;
+import static
io.trino.plugin.hive.HiveColumnHandle.isFileModifiedTimeColumnHandle;
+import static io.trino.plugin.hive.HiveColumnHandle.isFileSizeColumnHandle;
+import static io.trino.plugin.hive.HiveColumnHandle.isPartitionColumnHandle;
+import static io.trino.plugin.hive.HiveColumnHandle.isPathColumnHandle;
+import static io.trino.plugin.hive.util.HiveUtil.getPrefilledColumnValue;
+import static io.trino.spi.type.TypeUtils.writeNativeValue;
+
+/**
+ * Per-split constant values for the output columns that are not stored in the
data file: Hive-style
+ * partition columns and Trino's hidden metadata columns ({@code $path},
{@code $file_size},
+ * {@code $file_modified_time}, {@code $partition}). Value computation
delegates to
+ * {@link io.trino.plugin.hive.util.HiveUtil#getPrefilledColumnValue}, the
same implementation the Hive
+ * connector uses for these columns (including the {@code "\N"} hive-null
partition-value convention).
+ */
+public class PrefilledColumnValues
+{
+ private final Map<String, HivePartitionKey> partitionKeysByName;
+ private final String partitionName;
+ private final String filePath;
+ private final long fileSize;
+ private final long fileModifiedTime;
+
+ public static PrefilledColumnValues create(HudiSplit hudiSplit)
+ {
+ return new PrefilledColumnValues(hudiSplit);
+ }
+
+ private PrefilledColumnValues(HudiSplit hudiSplit)
+ {
+ List<HivePartitionKey> partitionKeys = hudiSplit.getPartitionKeys();
+ // ImmutableMap preserves insertion order, so $partition renders the
keys in the split's
+ // partition-column order.
+ ImmutableMap.Builder<String, HivePartitionKey> byName =
ImmutableMap.builder();
+ partitionKeys.forEach(partitionKey -> byName.put(partitionKey.name(),
partitionKey));
+ this.partitionKeysByName = byName.buildOrThrow();
+ this.partitionName = makePartName(
+ partitionKeys.stream().map(HivePartitionKey::name).toList(),
+ partitionKeys.stream().map(HivePartitionKey::value).toList());
+ // Parquet files will be prioritised over log files
+ HudiFile hudiFile = hudiSplit.getBaseFile().isPresent()
+ ? hudiSplit.getBaseFile().get()
+ : hudiSplit.getLogFiles().getFirst();
+ this.filePath = hudiFile.getPath();
+ this.fileSize = hudiFile.getFileSize();
+ this.fileModifiedTime = hudiFile.getModificationTime();
+ }
+
+ /**
+ * Returns whether this split can provide a value for the column, i.e. it
is a partition column of
+ * the split or a hidden metadata column Hudi populates.
+ */
+ public boolean isPrefilled(HiveColumnHandle columnHandle)
+ {
+ return partitionKeysByName.containsKey(columnHandle.getName())
+ || isPathColumnHandle(columnHandle)
+ || isFileSizeColumnHandle(columnHandle)
+ || isFileModifiedTimeColumnHandle(columnHandle)
+ || isPartitionColumnHandle(columnHandle);
+ }
+
+ /**
+ * Appends the column's value for this split to the builder; appends null
for a column this split
+ * cannot provide.
+ */
+ public void appendTo(HiveColumnHandle columnHandle, BlockBuilder
blockBuilder)
+ {
+ writeNativeValue(columnHandle.getType(), blockBuilder,
nativeValueOf(columnHandle));
+ }
+
+ /**
+ * Builds a run-length-encoded {@link Block} repeating the column's
constant value for
+ * {@code positionCount} positions (null-filled for a column this split
cannot provide).
+ */
+ public Block toRleBlock(HiveColumnHandle columnHandle, int positionCount)
+ {
+ return RunLengthEncodedBlock.create(columnHandle.getType(),
nativeValueOf(columnHandle), positionCount);
+ }
+
+ private Object nativeValueOf(HiveColumnHandle columnHandle)
+ {
+ if (!isPrefilled(columnHandle)) {
+ // Lenient null fill, e.g. for a hidden column Trino defines but
Hudi does not populate.
+ return null;
+ }
+ HivePartitionKey partitionKey =
partitionKeysByName.get(columnHandle.getName());
+ return getPrefilledColumnValue(
Review Comment:
The class javadoc note landed, but the Summary and Changelog still lists
three deltas without the zone change, and the round-2 behaviours are missing
from it too - a CUSTOM table with no persisted strategy id now fails with
`HUDI_BAD_DATA`, and under `hudi.parquet.use-column-names=false` a column
absent from the base file now null-fills instead of erroring. Worth folding all
of them in, since that text becomes the squashed commit message.
--
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]
