rohityadav1993 commented on code in PR #13031:
URL: https://github.com/apache/pinot/pull/13031#discussion_r1585398997
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartialUpsertHandler.java:
##########
@@ -84,7 +84,13 @@ private void setMergedValue(GenericRow row, String column,
@Nullable Object merg
row.putValue(column, mergedValue);
} else {
// if column exists but mapped to a null value then merger result was a
null value
- row.putDefaultNullValue(column,
_fieldSpecMap.get(column).getDefaultNullValue());
+ if (_fieldSpecMap.get(column).isSingleValueField()) {
+ row.putDefaultNullValue(column,
_fieldSpecMap.get(column).getDefaultNullValue());
+ } else {
+ // multivalue must necessarily use null or
MutableSegmentImpl.updateDictionary fails due to typecasting
+ // primitive to array
+ row.putDefaultNullValue(column, null);
Review Comment:
Typecasting is done assuming Object[] type in the code attached in
description(stacktrace). null can be typecasted but becuase defaultNullValue
are stored as primitive for multi value fields it fails when we persist
defualtNullValue when the merger result is null for multi value field.
Updating defaultNullValue behaviour for multi value will be major change and
introducing change typecasting will have a larger impact radius.
The fix should be safe as it still populates null vector and slightly urgent
as anyone merging null value for partial upsert would be seeing error records
and may not be aware if not observing the metrics.
--
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
