deemoliu commented on code in PR #13031:
URL: https://github.com/apache/pinot/pull/13031#discussion_r1585344830
##########
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:
i think this is a backward incompatible change, but it might be ok to change
it earlier since the impact of this change is not large, since this feature is
relatively new.
the problem is, the default value of pinot multi-value column, is the
default value of singleValue data type, which caused the type casting
exception. Generally there are two solutions
1. update the type casting code
2. Update the current **default null value** to `null` for multi-value
column, as shown in this diff.
@rohityadav1993 can you provide some insights on why #1 is not a feasible
solution.
--
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]
