Handling null cases in sum,min,max series builders #14084
Conversation
raghavyadav01
commented
Sep 25, 2024
raghavyadav01
commented
- Handling null cases in sum,min,max series builders.
- Tested and verified the changes.
...series-spi/src/main/java/org/apache/pinot/tsdb/spi/series/builders/MaxTimeSeriesBuilder.java
Outdated
Show resolved
Hide resolved
...series-spi/src/main/java/org/apache/pinot/tsdb/spi/series/builders/MaxTimeSeriesBuilder.java
Outdated
Show resolved
Hide resolved
ankitsultana
left a comment
ankitsultana
left a comment
There was a problem hiding this comment.
Thanks for addressing feedback. Let's make sure tests work though. Looks like some GH Outage right now.
Side-note: The code is most likely not good for performance and there may be better ways to do this. We can follow-up in Phase-3. From Uber side we plan to run a bunch of benchmarks too.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14084 +/- ##
============================================
+ Coverage 61.75% 63.98% +2.23%
- Complexity 207 1538 +1331
============================================
Files 2436 2596 +160
Lines 133233 143296 +10063
Branches 20636 21949 +1313
============================================
+ Hits 82274 91684 +9410
+ Misses 44911 44862 -49
- Partials 6048 6750 +702
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
...series-spi/src/main/java/org/apache/pinot/tsdb/spi/series/builders/MinTimeSeriesBuilder.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| @Override | ||
| public void addValueAtIndex(int timeBucketIndex, Double value) { |
There was a problem hiding this comment.
Suggest adding @Nullable annotation both to base class and the overrides
There was a problem hiding this comment.
Ack. Will add it in the next PR I am working on.
ankitsultana
added
the
timeseries-engine
4 participants
