Skip to content

[multistage] Change default limit to 10 for query.#11418

Closed
xiangfu0 wants to merge 1 commit intoapache:masterfrom
xiangfu0:default-limit-10
Closed

[multistage] Change default limit to 10 for query.#11418
xiangfu0 wants to merge 1 commit intoapache:masterfrom
xiangfu0:default-limit-10

Conversation

@xiangfu0
Copy link
Contributor

@xiangfu0 xiangfu0 commented Aug 23, 2023

  • Check parsed query for LogicalSort node and create it if not existed.

@xiangfu0 xiangfu0 force-pushed the default-limit-10 branch 11 times, most recently from 790eb7e to 30f6466 Compare August 25, 2023 23:46
@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2023

Codecov Report

Merging #11418 (e9c5946) into master (86a6b78) will decrease coverage by 0.03%.
The diff coverage is 85.00%.

@@             Coverage Diff              @@
##             master   #11418      +/-   ##
============================================
- Coverage     62.97%   62.95%   -0.03%     
+ Complexity     1111     1110       -1     
============================================
  Files          2319     2319              
  Lines        124394   124413      +19     
  Branches      18992    18998       +6     
============================================
- Hits          78340    78322      -18     
- Misses        40489    40523      +34     
- Partials       5565     5568       +3     
Flag Coverage Δ
integration <0.01% <0.00%> (ø)
integration1 <0.01% <0.00%> (ø)
integration2 0.00% <0.00%> (ø)
java-11 62.92% <85.00%> (+0.02%) ⬆️
java-17 62.81% <85.00%> (+<0.01%) ⬆️
java-20 62.80% <85.00%> (-0.04%) ⬇️
temurin 62.95% <85.00%> (-0.03%) ⬇️
unittests 62.94% <85.00%> (-0.03%) ⬇️
unittests1 67.46% <85.00%> (-0.01%) ⬇️
unittests2 14.43% <0.00%> (-0.03%) ⬇️

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

Files Changed Coverage Δ
.../java/org/apache/pinot/query/QueryEnvironment.java 89.70% <85.00%> (-0.90%) ⬇️

... and 10 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xiangfu0 xiangfu0 force-pushed the default-limit-10 branch 2 times, most recently from 729a6c7 to 0587878 Compare August 26, 2023 23:04
Copy link
Contributor

@walterddr walterddr left a comment

Choose a reason for hiding this comment

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

i am not sure this is the right way to handle un-limited queries. it make sense to rewrite the query in simple single-table way but this might back-fire.

if the goal is not to blow up when un-reasonably large queries are issues. we should directly set the limit similar to #11401

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants