-
Notifications
You must be signed in to change notification settings - Fork 139
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
Improve pushdown optimization and logical to physical transformation #1091
Conversation
Signed-off-by: Chen Dai <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 2.x #1091 +/- ##
============================================
- Coverage 98.31% 95.78% -2.53%
- Complexity 3485 3502 +17
============================================
Files 348 350 +2
Lines 8707 9306 +599
Branches 555 669 +114
============================================
+ Hits 8560 8914 +354
- Misses 142 334 +192
- Partials 5 58 +53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Is it related to #1088? |
Signed-off-by: Chen Dai <[email protected]>
Signed-off-by: Chen Dai <[email protected]>
Signed-off-by: Chen Dai <[email protected]>
Signed-off-by: Chen Dai <[email protected]>
Signed-off-by: Chen Dai <[email protected]>
Signed-off-by: Chen Dai <[email protected]>
Signed-off-by: Chen Dai <[email protected]>
Signed-off-by: Chen Dai <[email protected]>
Signed-off-by: Chen Dai <[email protected]>
Signed-off-by: Chen Dai <[email protected]>
Signed-off-by: Chen Dai <[email protected]>
I like the PR description. Can you add it to the docs section? |
Sure, I'm drafting another PR for updating our dev docs: #1092. Will add this too. Thanks! |
core/src/main/java/org/opensearch/sql/storage/read/TableScanBuilder.java
Show resolved
Hide resolved
core/src/main/java/org/opensearch/sql/planner/optimizer/LogicalPlanOptimizer.java
Show resolved
Hide resolved
ML integration seems missing IT. Just tested manually to confirm it is not impacted. In particular, |
Signed-off-by: Chen Dai [email protected]
Background
This section introduces the current architecture of logical optimizer and physical transformation.
Logical-to-Logical Optimization
Currently each storage engine adds its own logical operator as concrete implementation for
TableScanOperator
abstraction. Typically each data source needs to add 2 logical operators for table scan with and without aggregation. Take OpenSearch for example, there areOpenSearchLogicalIndexScan
andOpenSearchLogicalIndexAgg
and a bunch of pushdown optimization rules for each accordingly.Logical-to-Physical Transformation
After logical transformation, planner will let the
Table
inLogicalRelation
(identified before logical transformation above) transform the logical plan to physical plan.Problem Statement
The current planning architecture causes 2 serious problems:
PrometheusLogicalMetricAgg
andPrometheusLogicalMetricScan
accordingly. They have the exactly same pattern to match query plan tree as OpenSearch.Table
s in query plan. Because only 1 of them has the chance to do theimplement()
. This is a blocker for supportingINSERT ... SELECT ...
statement or JOIN query. See code below.Solution
TableScanBuilder
A new abstraction
TableScanBuilder
is added as a transition operator during logical planning and optimization. Each data source provides its implementation class byTable
interface. The push down difference in non-aggregate and aggregate query is hidden inside specific scan builder, for exampleOpenSearchIndexScanBuilder
rather than exposed to core module.TablePushDownRules
In this way,
LogicalOptimizier
in core module always have the same set of rule for all push down optimization.Examples
The following diagram illustrates how
TableScanBuilder
along withTablePushDownRule
solve the problem aforementioned.Similarly,
TableWriteBuilder
will be added and work in the same way in separate PR: #1094TODO
optimize()
andimplement()
if item 1 and 2 completepushdownHighlight
once it's moved to OpenSearch storageTableScanOperator
to the newread
package (leave it in this PR to avoid even more file changed)Issues Resolved
#948
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.