Skip to content

Conversation

@mr-agrwal
Copy link
Contributor

Description

Adding aggregate function to return first value of time-based data set.

Upgrade Notes

Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)

  • Yes (Please label as backward-incompat, and complete the section below on Release Notes)

Does this PR fix a zero-downtime upgrade introduced earlier?

  • Yes (Please label this as backward-incompat, and complete the section below on Release Notes)

Does this PR otherwise need attention when creating release notes? Things to consider:

  • New configuration options
  • Deprecation of configurations
  • Signature changes to public methods/interfaces
  • New plugins added or old plugins removed
  • Yes (Please label this PR as release-notes and complete the section on Release Notes)

Release Notes

add FIRSTWITHTIME aggregate function support

Documentation

Will raise an another PR to update the documentation once this PR is approved and the changes are finalized.

@Jackie-Jiang
Copy link
Contributor

The new added tests are failing. Can you please rebase to the latest master? The quickstart test failure is not related to this PR

mr-agrwal added 4 commits May 4, 2022 15:29
Update test to remove pql
Fix default value pairs
Fix Group by query expected result
Fix String function call
@codecov-commenter
Copy link

codecov-commenter commented May 4, 2022

Codecov Report

Merging #8181 (41a107e) into master (43e1298) will increase coverage by 0.10%.
The diff coverage is 85.09%.

@@             Coverage Diff              @@
##             master    #8181      +/-   ##
============================================
+ Coverage     69.49%   69.60%   +0.10%     
+ Complexity     4412     4411       -1     
============================================
  Files          1667     1673       +6     
  Lines         87320    87595     +275     
  Branches      13086    13127      +41     
============================================
+ Hits          60687    60968     +281     
+ Misses        22401    22382      -19     
- Partials       4232     4245      +13     
Flag Coverage Δ
integration1 27.70% <0.00%> (-0.03%) ⬇️
unittests1 66.49% <85.09%> (+0.06%) ⬆️
unittests2 14.28% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ion/function/FirstWithTimeAggregationFunction.java 49.27% <49.27%> (ø)
...gregation/function/AggregationFunctionFactory.java 80.91% <60.00%> (-2.71%) ⬇️
...n/FirstDoubleValueWithTimeAggregationFunction.java 100.00% <100.00%> (ø)
...on/FirstFloatValueWithTimeAggregationFunction.java 100.00% <100.00%> (ø)
...tion/FirstIntValueWithTimeAggregationFunction.java 100.00% <100.00%> (ø)
...ion/FirstLongValueWithTimeAggregationFunction.java 100.00% <100.00%> (ø)
...n/FirstStringValueWithTimeAggregationFunction.java 100.00% <100.00%> (ø)
...che/pinot/segment/spi/AggregationFunctionType.java 89.41% <100.00%> (+0.12%) ⬆️
...ller/helix/core/minion/TaskTypeMetricsUpdater.java 80.00% <0.00%> (-20.00%) ⬇️
...pinot/core/query/request/context/TimerContext.java 91.66% <0.00%> (-4.17%) ⬇️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43e1298...41a107e. Read the comment docs.

@Jackie-Jiang Jackie-Jiang added release-notes Referenced by PRs that need attention when compiling the next release notes feature labels May 4, 2022
@Jackie-Jiang Jackie-Jiang merged commit b317246 into apache:master May 4, 2022
@mr-agrwal mr-agrwal deleted the firstwithtime branch May 5, 2022 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature release-notes Referenced by PRs that need attention when compiling the next release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants