ankitsultana commented on code in PR #14084:
URL: https://github.com/apache/pinot/pull/14084#discussion_r1775768452
##########
pinot-timeseries/pinot-timeseries-spi/src/main/java/org/apache/pinot/tsdb/spi/series/builders/MaxTimeSeriesBuilder.java:
##########
@@ -39,14 +40,15 @@ public MaxTimeSeriesBuilder(String id, TimeBuckets
timeBuckets, List<String> tag
@Override
public void addValueAtIndex(int timeBucketIndex, Double value) {
- if (_values[timeBucketIndex] == null || value > _values[timeBucketIndex]) {
- _values[timeBucketIndex] = value;
+ Double expectedValue = (value == null) ? (Double) 0.0 : value;
Review Comment:
Using 0.0 as default is incorrect behavior imo because you can have negative
and null values together.
If the existing value is null, we can simply return. (similar comment for
other builders)
##########
pinot-timeseries/pinot-timeseries-spi/src/main/java/org/apache/pinot/tsdb/spi/series/builders/MaxTimeSeriesBuilder.java:
##########
@@ -39,14 +40,15 @@ public MaxTimeSeriesBuilder(String id, TimeBuckets
timeBuckets, List<String> tag
@Override
public void addValueAtIndex(int timeBucketIndex, Double value) {
- if (_values[timeBucketIndex] == null || value > _values[timeBucketIndex]) {
- _values[timeBucketIndex] = value;
+ Double expectedValue = (value == null) ? (Double) 0.0 : value;
+ if (_values[timeBucketIndex] == null || expectedValue >
_values[timeBucketIndex]) {
+ _values[timeBucketIndex] = expectedValue;
}
}
@Override
public void addValue(long timeValue, Double value) {
- int timeBucketIndex = _timeBuckets.resolveIndex(timeValue);
+ int timeBucketIndex =
Objects.requireNonNull(_timeBuckets).resolveIndex(timeValue);
Review Comment:
Can we avoid the requireNonNull? It will anyways lead to the same error
message (NullPointerException), and it will add another function call to a hot
part of the code.
--
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]
