Copilot commented on code in PR #17113:
URL: https://github.com/apache/pinot/pull/17113#discussion_r2488269222
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java:
##########
@@ -1193,10 +1220,12 @@ private void
createDerivedColumnForwardIndexWithoutDictionary(String column, Fie
forwardIndexCreator.putBytesMV((byte[][]) outputValue);
break;
default:
- throw new IllegalStateException();
+ throw new IllegalStateException(
+ "Unsupported data type: " + fieldSpec.getDataType() + ", for
value: " + outputValue);
}
}
}
+ forwardIndexCreator.seal();
Review Comment:
The `forwardIndexCreator.seal()` call is placed outside both the
single-value and multi-value conditional blocks, causing it to execute
regardless of whether single-value or multi-value logic ran. For MAP type
(which uses the single-value branch and calls `add()` at line 1193), this is
correct. However, this changes behavior for all data types by adding a seal
call that wasn't there before. This should be reviewed to ensure it doesn't
cause issues with other data types that may handle sealing differently.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java:
##########
@@ -51,14 +55,15 @@
* heterogeneous value types for a key are encountered will construct the Map
statistics it can be raised as a fault.
*/
public class MapColumnPreIndexStatsCollector extends
AbstractColumnStatisticsCollector {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(MapColumnPreIndexStatsCollector.class);
private final Object2ObjectOpenHashMap<String,
AbstractColumnStatisticsCollector> _keyStats =
new Object2ObjectOpenHashMap<>(INITIAL_HASH_SET_SIZE);
private final Map<String, Integer> _keyFrequencies = new
Object2ObjectOpenHashMap<>(INITIAL_HASH_SET_SIZE);
private String[] _sortedKeys;
private int _minLength = Integer.MAX_VALUE;
private int _maxLength = 0;
private boolean _sealed = false;
- private ComplexFieldSpec _colFieldSpec;
+ private final ComplexFieldSpec _colFieldSpec;
Review Comment:
The field `_colFieldSpec` is marked as final but is never initialized in the
constructor, which will cause a compilation error. This field needs to be
initialized when constructed or the final modifier should be removed.
--
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]
