Skip to content

Conversation

@xiangfu0
Copy link
Contributor

@xiangfu0 xiangfu0 commented Mar 31, 2022

Description

Rewrite PinotQuery based on expression hints at segment level.

Split the query rewrite logic from Timestamp index: #8343

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

Rewrite PinotQuery based on Expression hints at segment level at Pinot Instance query executor.

Documentation

@xiangfu0 xiangfu0 added the release-notes Referenced by PRs that need attention when compiling the next release notes label Mar 31, 2022
@xiangfu0 xiangfu0 changed the title Rewrite PinotQuery based on expression hints at segment level Rewrite PinotQuery based on expression hints at instance/segment level Mar 31, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2022

Codecov Report

Merging #8451 (85dd906) into master (370d86c) will decrease coverage by 7.04%.
The diff coverage is 82.85%.

❗ Current head 85dd906 differs from pull request most recent head f2637f9. Consider uploading reports for the commit f2637f9 to get more accurate results

@@             Coverage Diff              @@
##             master    #8451      +/-   ##
============================================
- Coverage     70.63%   63.59%   -7.05%     
+ Complexity     4282     4198      -84     
============================================
  Files          1663     1652      -11     
  Lines         87305    86987     -318     
  Branches      13216    13189      -27     
============================================
- Hits          61671    55319    -6352     
- Misses        21350    27616    +6266     
+ Partials       4284     4052     -232     
Flag Coverage Δ
integration1 27.18% <35.71%> (+0.15%) ⬆️
integration2 26.10% <35.71%> (+0.25%) ⬆️
unittests1 67.04% <82.85%> (-0.01%) ⬇️
unittests2 ?

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

Impacted Files Coverage Δ
...ot/common/request/context/predicate/Predicate.java 100.00% <ø> (ø)
...pinot/core/plan/maker/InstancePlanMakerImplV2.java 74.82% <69.23%> (-2.18%) ⬇️
...ommon/request/context/predicate/BasePredicate.java 100.00% <100.00%> (ø)
.../common/request/context/predicate/EqPredicate.java 61.53% <100.00%> (-5.13%) ⬇️
.../common/request/context/predicate/InPredicate.java 72.22% <100.00%> (-2.78%) ⬇️
.../request/context/predicate/IsNotNullPredicate.java 63.63% <100.00%> (-5.60%) ⬇️
...mon/request/context/predicate/IsNullPredicate.java 63.63% <100.00%> (-5.60%) ⬇️
.../request/context/predicate/JsonMatchPredicate.java 53.84% <100.00%> (-6.16%) ⬇️
...mmon/request/context/predicate/NotEqPredicate.java 61.53% <100.00%> (-5.13%) ⬇️
...mmon/request/context/predicate/NotInPredicate.java 72.22% <100.00%> (-2.78%) ⬇️
... and 234 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 370d86c...f2637f9. Read the comment docs.

@xiangfu0 xiangfu0 force-pushed the expression-hints-server-override branch from 24574f9 to f37146c Compare March 31, 2022 21:49
@xiangfu0 xiangfu0 force-pushed the expression-hints-server-override branch 4 times, most recently from fefc640 to 7aaa74d Compare April 1, 2022 10:06
Copy link
Member

@richardstartin richardstartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@xiangfu0 xiangfu0 force-pushed the expression-hints-server-override branch from 7aaa74d to 11f01a7 Compare April 1, 2022 18:35
@xiangfu0 xiangfu0 requested a review from Jackie-Jiang April 1, 2022 18:55
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM otherwise

@xiangfu0 xiangfu0 force-pushed the expression-hints-server-override branch from 11f01a7 to 2a10251 Compare April 1, 2022 20:15
@xiangfu0 xiangfu0 force-pushed the expression-hints-server-override branch from 2a10251 to 4bfb2df Compare April 1, 2022 20:26
@xiangfu0 xiangfu0 force-pushed the expression-hints-server-override branch from 4bfb2df to 292e9de Compare April 1, 2022 21:03
@xiangfu0 xiangfu0 force-pushed the expression-hints-server-override branch from 292e9de to f2637f9 Compare April 1, 2022 21:52
@xiangfu0 xiangfu0 merged commit 1f5c4cf into apache:master Apr 1, 2022
@xiangfu0 xiangfu0 deleted the expression-hints-server-override branch April 1, 2022 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

5 participants