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: 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