Jackie-Jiang commented on code in PR #19501:
URL: https://github.com/apache/pinot/pull/19501#discussion_r4079591943
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -239,7 +236,7 @@ private DimensionTable createFastLookupDimensionTable() {
return null;
}
- Object[] primaryKey = recordReader.getRecordValues(i, pkIndexes);
+ Object[] primaryKey = recordReader.getPrimaryKeys(i);
Review Comment:
In preload mode this stores the BYTES key as `ByteArray`. When the requested
value is that primary-key column, `FastLookupDimensionTable.getValue()` returns
the `ByteArray` from the probe. `LookupTransformFunction.setBytesSV()` accepts
only `byte[]`, so `LOOKUP('dimAsset', 'assetId', ...)` returns `EMPTY_BYTES`
for a matching row. Please unwrap the wrapper at the query boundary (or
normalize the lookup result) and add a query-level regression test; the
direct-manager test does not exercise this conversion.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/readers/PinotSegmentRecordReader.java:
##########
@@ -357,4 +402,22 @@ public void close()
throw closeException;
}
}
+
+ public Object[] getPrimaryKeys(int docId) {
+ int[] primaryKeyIndexes = getIndexesForColumns(_primaryKeyColumns);
Review Comment:
Both dimension-table modes call this once per document.
`getIndexesForColumns` allocates an `int[]` and runs `_columnNames.indexOf` for
every PK column, so the same column search now repeats for every row instead of
once per segment. Please cache the PK indexes after the reader registers its
columns and reuse them here.
##########
pinot-core/src/test/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataBytesPrimaryKeyTest.java:
##########
@@ -0,0 +1,249 @@
+package org.apache.pinot.core.data.manager.offline;
Review Comment:
Please add the ASF license header to this new Java file. The current Pinot
Linter job fails `license:check` for `pinot-core`, and Unit Test Set 1 fails
RAT with one unapproved license; the tests never start.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
