-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PoC fetch all #697
PoC fetch all #697
Conversation
Signed-off-by: Sean Kao <[email protected]>
Signed-off-by: Sean Kao <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #697 +/- ##
=============================================
- Coverage 94.74% 62.76% -31.99%
=============================================
Files 283 10 -273
Lines 7676 658 -7018
Branches 561 119 -442
=============================================
- Hits 7273 413 -6860
+ Misses 349 192 -157
+ Partials 54 53 -1
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
if (node.getCommandType() != RareTopN.CommandType.TOPOFALL) { | ||
return new LogicalRareTopN(child, node.getCommandType(), noOfResults, fields, groupBys); | ||
} else { | ||
return new LogicalRareTopN(child, RareTopN.CommandType.TOP, noOfResults, fields, groupBys); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only made minimal changes for the RareTopN
to make topOfAll
work, so here it may look ugly. There's no LogicalRareTopN
for topOfAll
. topOfAll
is treated as a top
command logically. They have the same logical plan and differ only in the planContext. This leads to different physical plan generation.
The RareTopN.CommandType.TOPOFALL
is merely for this class to know whether to setIndexScanType
as in line 245
Description
PoC for using PlanContext during LogicalPlan generation
Analyzer.analyze(...)
to decide whether to 1) fetch all data using scrolling, or 2) fetch data via regular request bounded by the query size limit.This is useful for ML commands.
I added a
topOfAll
command in this PR that's purely for demo purpose. It's almost identical to thetop
command, except thattop
returns the top results of the first 200 rows, whiletopOfAll
returns the top results of all rows.Sample queries using the opensearch_dashboards_sample_data_flights:
Explain (for OpenSearchIndexScan, one uses OpenSearchQueryRequest, while the other uses OpenSearchScrollRequest):
Issues Resolved
#656
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.