voonhous commented on code in PR #19483:
URL: https://github.com/apache/hudi/pull/19483#discussion_r3705584696
##########
hudi-trino/src/main/java/io/trino/plugin/hudi/util/HudiAvroSerializer.java:
##########
@@ -170,21 +177,36 @@ public Object getValue(Page sourcePage, int channel, int
position)
public void buildRecordInPage(PageBuilder pageBuilder, IndexedRecord
record)
{
pageBuilder.declarePosition();
- int blockSeq = 0;
- for (int channel = 0; channel < columnTypes.size(); channel++,
blockSeq++) {
- BlockBuilder output = pageBuilder.getBlockBuilder(blockSeq);
- HiveColumnHandle columnHandle = columnHandles.get(channel);
- if (prefilledColumnValues.isPrefilled(columnHandle)) {
- prefilledColumnValues.appendTo(columnHandle, output);
+ // Record may not be projected, get field positions from its own schema
+ int[] fieldPositions = fieldPositionsFor(record.getSchema());
+ for (int channel = 0; channel < columnTypes.size(); channel++) {
+ BlockBuilder output = pageBuilder.getBlockBuilder(channel);
+ int fieldPosition = fieldPositions[channel];
+ if (fieldPosition < 0) {
+ prefilledColumnValues.appendTo(columnHandles.get(channel),
output);
Review Comment:
Not out of scope -- fixed in #19495. `nativeValueOf` now memoizes the
resolved value per column, so each one is resolved once per split instead of
once per record.
One correction to the framing: `HudiBaseFileOnlyPageSource` wasn't getting
it once per page via caching. `toRleBlock` recomputes per page too and just
amortizes over the block through RLE. So there was no existing memo to mirror
-- which makes the case stronger, since this helps both paths (MOR per-record
to once, COW per-page to once).
Two traps worth flagging in the diff: keyed on the column name rather than
the handle, because `HiveColumnHandle.hashCode` hashes seven fields through a
varargs array; and `containsKey` rather than a null check, because `null` is a
legitimate resolved value (hive-`\N`, plus the lenient fallback for a column
the split cannot provide). That's also why it's a `HashMap` and not a
`ConcurrentHashMap`.
--
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]
