xiangfu0 commented on code in PR #19118:
URL: https://github.com/apache/pinot/pull/19118#discussion_r3689214998
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -311,6 +313,9 @@ public void addSegment(ImmutableSegment segment) {
/// Creates a RecordInfoReader for the given segment. When comparison
columns are configured, reads comparison values
/// from the columns. When comparison columns are empty, uses segment
creation time as the comparison value.
protected UpsertUtils.RecordInfoReader createRecordInfoReader(IndexSegment
segment) {
+ if (usesImplicitComparisonTime()) {
+ return new UpsertUtils.RecordInfoReader(segment, _primaryKeyColumns,
_deleteRecordColumn);
+ }
Review Comment:
Fixed in `37710d51dd`. Legacy segments without a physical
`_upsert_comparison_ts` data source now use `ConstantComparisonColumnReader`,
so the fallback `Long` is created once and reused. The new two-document
regression asserts both the source creation time and object identity reuse.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/readers/PinotSegmentRecordReader.java:
##########
@@ -245,6 +263,14 @@ public void getRecord(int docId, GenericRow buffer) {
for (Map.Entry<String, PinotSegmentColumnReader> entry :
_columnReaderMap.entrySet()) {
String column = entry.getKey();
PinotSegmentColumnReader columnReader = entry.getValue();
+ if (column.equals(DEFAULT_UPSERT_COMPARISON_TIME_COLUMN)) {
+ Object comparisonTime = columnReader.isNull(docId) ? null :
columnReader.getValue(docId);
+ if (comparisonTime == null || ((Number) comparisonTime).longValue() ==
Long.MIN_VALUE) {
+ comparisonTime =
_indexSegment.getSegmentMetadata().getIndexCreationTime();
+ }
+ buffer.putValue(column, comparisonTime);
+ continue;
+ }
Review Comment:
Fixed in `37710d51dd`. The table-aware record-reader path now reads
`_upsert_comparison_ts` with primitive `getLong()` and handles null/unset
values without `Object` or `Number` casts. The feature path is also separated
from the common per-column loop.
--
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]
