klsince commented on code in PR #15107:
URL: https://github.com/apache/pinot/pull/15107#discussion_r1975904977
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/merger/PartialUpsertColumnarMerger.java:
##########
@@ -74,7 +75,10 @@ public void merge(LazyRow previousRow, GenericRow newRow,
Map<String, Object> re
// (1) If the value of the previous is null value, skip merging and use
the new value
// (2) Else If the value of new value is null, use the previous value
(even for comparison columns)
// (3) Else If the column is not a comparison column, we applied the
merged value to it
- if (!(merger instanceof OverwriteMerger)) {
+ if (merger instanceof ForceOverwriteMerger) {
+ Object prevValue = previousRow.getValue(column);
+ resultHolder.put(column, merger.merge(prevValue,
newRow.getValue(column)));
Review Comment:
is there need to check `if (prevValue != null)` like the OverwriteMerger
below? or does merger.merge() already handle null prevValue cleanly
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
