Copilot commented on code in PR #16254:
URL: https://github.com/apache/pinot/pull/16254#discussion_r2193557054
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/GenericRow.java:
##########
@@ -100,7 +80,7 @@ public void init(GenericRow row) {
* the value for the field can be {@code null}.
*/
public Map<String, Object> getFieldToValueMap() {
- return Collections.unmodifiableMap(_fieldToValueMap);
+ return _fieldToValueMap;
Review Comment:
[nitpick] Returning the internal `_fieldToValueMap` allows callers to mutate
the row’s internal state. Consider returning an unmodifiable view or a
defensive copy to preserve encapsulation.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/FilterTransformer.java:
##########
@@ -56,16 +61,20 @@ public boolean isNoOp() {
@Override
public List<String> getInputColumns() {
- return _evaluator != null ? _evaluator.getArguments() : List.of();
+ assert _evaluator != null;
+ return _evaluator.getArguments();
}
@Override
- public GenericRow transform(GenericRow record) {
- if (_evaluator != null) {
+ public List<GenericRow> transform(List<GenericRow> records) {
+ assert _evaluator != null;
Review Comment:
Using `assert _evaluator != null` will cause an AssertionError when no
filter is configured. Guard against null `_evaluator` or skip registering the
transformer when no filter function is provided.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/GenericRow.java:
##########
@@ -109,7 +89,7 @@ public Map<String, Object> getFieldToValueMap() {
* {@link #putDefaultNullValue(String, Object)}.
*/
public Set<String> getNullValueFields() {
- return Collections.unmodifiableSet(_nullValueFields);
+ return _nullValueFields;
Review Comment:
[nitpick] Returning the internal `_nullValueFields` set exposes it for
external modification. Consider returning an unmodifiable set or cloning to
prevent unintended mutations.
```suggestion
return Collections.unmodifiableSet(_nullValueFields);
```
--
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]
