Copilot commented on code in PR #19014:
URL: https://github.com/apache/pinot/pull/19014#discussion_r3611318135
##########
helm/pinot/values.yaml:
##########
@@ -29,9 +29,9 @@ image:
# - `release-x.y.z` or `x.y.z` tags are available for each release, e.g.
release-1.0.0, release-0.12.1, 1.0.0, 0.12.1, etc.
#
# Default JDK comes with Amazon Corretto 21, here are also images with
supported JDK 21 variants:
- # - Amazon Corretto 21, e.g. `latest`, `latest-21`, `1.0.0-21`,
`latest-21-amazoncorretto`,
- # `1.0.0-21-amazoncorretto`
- # - MS OpenJDK 21, e.g. `latest-21-ms-openjdk`, `1.0.0-21-ms-openjdk`
+ # - Amazon Corretto 25, e.g. `latest`, `latest-25`, `1.0.0-25`,
`latest-25-amazoncorretto`,
+ # `1.0.0-25-amazoncorretto`
+ # - MS OpenJDK 25, e.g. `latest-25-ms-openjdk`, `1.0.0-25-ms-openjdk`
Review Comment:
The image tag guidance still says the default image is Amazon Corretto 21 /
“supported JDK 21 variants”, but the examples and this PR’s intent are JDK 25
(and the PR description states `latest` promotes 25-ms-openjdk). Update the
comment line to match the new default and version.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/IntegerTupleSketchValueAggregator.java:
##########
@@ -100,15 +101,15 @@ private Sketch extractSketch(Object value) {
@Override
public Object applyRawValue(Object aggregatedValue, byte[] rawValue) {
- Union tupleUnion = extractUnion(aggregatedValue);
+ TupleUnion tupleUnion = extractUnion(aggregatedValue);
tupleUnion.union(deserializeAggregatedValue(rawValue));
return tupleUnion;
}
@Override
public Object applyAggregatedValue(Object value, Object aggregatedValue) {
- Union tupleUnion = extractUnion(aggregatedValue);
- Sketch sketch = extractSketch(value);
+ TupleUnion tupleUnion = extractUnion(aggregatedValue);
+ TupleSketch sketch = extractSketch(value);
Review Comment:
These locals can keep their generic types (`IntegerSummary`) to avoid
rawtype warnings and accidental misuse.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/IntegerTupleSketchValueAggregator.java:
##########
@@ -62,23 +62,24 @@ public DataType getAggregatedValueType() {
@Override
public Object getInitialAggregatedValue(@Nullable byte[] rawValue) {
- Union tupleUnion = new Union<>(_nominalEntries, new
IntegerSummarySetOperations(_mode, _mode));
+ TupleUnion tupleUnion = new TupleUnion<>(_nominalEntries, new
IntegerSummarySetOperations(_mode, _mode));
Review Comment:
Avoid using raw `TupleUnion` here; it drops the `IntegerSummary` type
information and introduces unchecked warnings. Use `TupleUnion<IntegerSummary>`
consistently.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/IntegerTupleSketchValueAggregator.java:
##########
@@ -145,12 +146,12 @@ public int getMaxAggregatedValueByteSize() {
@Override
public byte[] serializeAggregatedValue(Object value) {
- Sketch sketch = extractSketch(value);
+ TupleSketch sketch = extractSketch(value);
Review Comment:
Use the generic `TupleSketch<IntegerSummary>` type here instead of the raw
`TupleSketch` to keep type-safety and avoid unchecked warnings.
##########
docker/images/pinot-base/README.md:
##########
@@ -36,19 +36,19 @@ The build shell is:
For Amazon Corretto 21:
Review Comment:
This section header still says “For Amazon Corretto 21” while the build
commands below are now for `:25-amazoncorretto`. Update the header to avoid
confusing readers.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/aggregator/IntegerTupleSketchValueAggregator.java:
##########
@@ -100,15 +101,15 @@ private Sketch extractSketch(Object value) {
@Override
public Object applyRawValue(Object aggregatedValue, byte[] rawValue) {
- Union tupleUnion = extractUnion(aggregatedValue);
+ TupleUnion tupleUnion = extractUnion(aggregatedValue);
Review Comment:
Avoid raw `TupleUnion` in the hot-path update; use the generic type to
prevent unchecked operations from leaking into callers.
--
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]
