Skip to content

Conversation

@xiangfu0
Copy link
Contributor

@xiangfu0 xiangfu0 commented Aug 14, 2023

Per #11251 , create two new Broker API endpoints for multi-stage query engine.

GET /query
POST /query

Make Integration tests to use new endpoint for testing multi-stage engine.

@xiangfu0 xiangfu0 force-pushed the 11251-create-a-separated-query-endpoint-for-multi-stage-query-engine branch from 2c08ee3 to 0379ba5 Compare August 14, 2023 07:28
@xiangfu0 xiangfu0 added the multi-stage Related to the multi-stage query engine label Aug 14, 2023
@xiangfu0 xiangfu0 force-pushed the 11251-create-a-separated-query-endpoint-for-multi-stage-query-engine branch from 0379ba5 to 1e614e2 Compare August 14, 2023 07:45
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.

Seems there is a bug in MultiStageBrokerRequestHandler where it uses TRACE in the json request instead of the query options. We should use the one in query options instead. RequestUtils.setOptions() already set the one in json request into the query options.
Other than that, LGTM

@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2023

Codecov Report

Merging #11341 (f24e48c) into master (f25f3ed) will decrease coverage by 0.16%.
Report is 2 commits behind head on master.
The diff coverage is 39.58%.

@@             Coverage Diff              @@
##             master   #11341      +/-   ##
============================================
- Coverage     68.46%   68.31%   -0.16%     
+ Complexity     6535     6532       -3     
============================================
  Files          2233     2233              
  Lines        119952   119971      +19     
  Branches      18191    18191              
============================================
- Hits          82128    81955     -173     
- Misses        31984    32171     +187     
- Partials       5840     5845       +5     
Flag Coverage Δ
integration1 20.86% <37.50%> (-0.13%) ⬇️
integration2 25.63% <39.58%> (-0.03%) ⬇️
java-11 33.92% <39.58%> (-34.49%) ⬇️
java-17 67.69% <39.58%> (-0.61%) ⬇️
java-20 65.86% <37.50%> (-2.44%) ⬇️
temurin 68.31% <39.58%> (-0.16%) ⬇️
unittests1 67.10% <ø> (-0.22%) ⬇️
unittests2 14.65% <8.33%> (-0.01%) ⬇️

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

Files Changed Coverage Δ
...pinot/broker/api/resources/PinotClientRequest.java 40.18% <27.27%> (-5.76%) ⬇️
...inot/client/JsonAsyncHttpPinotClientTransport.java 51.61% <58.33%> (+9.50%) ⬆️
...requesthandler/MultiStageBrokerRequestHandler.java 69.23% <100.00%> (+0.52%) ⬆️
...ient/JsonAsyncHttpPinotClientTransportFactory.java 75.00% <100.00%> (+0.64%) ⬆️

... and 31 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 11251-create-a-separated-query-endpoint-for-multi-stage-query-engine branch from 1e614e2 to b3e6817 Compare August 14, 2023 10:04
@xiangfu0 xiangfu0 force-pushed the 11251-create-a-separated-query-endpoint-for-multi-stage-query-engine branch from b3e6817 to f24e48c Compare August 14, 2023 18:35
@xiangfu0 xiangfu0 merged commit 8318650 into apache:master Aug 14, 2023
@xiangfu0 xiangfu0 deleted the 11251-create-a-separated-query-endpoint-for-multi-stage-query-engine branch August 14, 2023 19:44
@Jackie-Jiang Jackie-Jiang added rest-api release-notes Referenced by PRs that need attention when compiling the next release notes labels Aug 15, 2023
@Jackie-Jiang
Copy link
Contributor

Please add these new APIs to the pinot doc

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

Labels

multi-stage Related to the multi-stage query engine release-notes Referenced by PRs that need attention when compiling the next release notes rest-api

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants