-
Notifications
You must be signed in to change notification settings - Fork 179
Support partial filter push down #3850
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
Support partial filter push down #3850
Conversation
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
|
ping @LantaoJin @penghuo |
| { | ||
| "calcite": { | ||
| "logical": "LogicalProject(age=[$8], address=[$2])\n LogicalFilter(condition=[AND(>=($8, 1), =($2, '880 Holmes Lane'))])\n CalciteLogicalIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]])\n", | ||
| "physical": "EnumerableCalc(expr#0..1=[{inputs}], expr#2=['880 Holmes Lane':VARCHAR], expr#3=[=($t0, $t2)], age=[$t1], address=[$t0], $condition=[$t3])\n CalciteEnumerableIndexScan(table=[[OpenSearch, opensearch-sql_test_index_account]], PushDownContext=[[PROJECT->[address, age], FILTER->AND(>=($1, 1), =($0, '880 Holmes Lane'))], OpenSearchRequestBuilder(sourceBuilder={\"from\":0,\"timeout\":\"1m\",\"query\":{\"bool\":{\"must\":[{\"range\":{\"age\":{\"from\":1,\"to\":null,\"include_lower\":true,\"include_upper\":true,\"boost\":1.0}}}],\"adjust_pure_negative\":true,\"boost\":1.0}},\"_source\":{\"includes\":[\"address\",\"age\"],\"excludes\":[]},\"sort\":[{\"_doc\":{\"order\":\"asc\"}}]}, requestedTotalSize=2147483647, pageSize=null, startFrom=0)])\n" |
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.
shouldn't be FILTER->[>=($1, 1)] instead of FILTER->AND(>=($1, 1), =($0, '880 Holmes Lane')) in PushDownContext?
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.
It uses the original condition currently as its digest now. If we needs to use the pushed condition as its digest, we need to store that RexNode as well like non-pushed condition. Both is ok for functionality, the latter one should be more appropriate for explanation.
Will make that change.
Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
|
Find a good case for Calcite with partial push down: Calcite with partial down could execute this PPL while v2 will throw exception: It's because Calcite could only push the relevance function into scan through |
Signed-off-by: Heng Qian <qianheng@amazon.com>
opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java
Show resolved
Hide resolved
| if (queryExpression.isPartial()) { | ||
| // Only CompoundQueryExpression could be partial. | ||
| List<RexNode> conditions = queryExpression.getUnAnalyzableNodes(); | ||
| RexNode newCondition = constructCondition(conditions, getCluster().getRexBuilder()); | ||
| return filter.copy(filter.getTraitSet(), newScan, newCondition); | ||
| } |
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.
nit, pushDownFilter should return newScan, if it is partial, return pair of newScan and un-pushed filters, then let IndexScanRule create new Filter node?
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.
Yeah
…ushDown # Conflicts: # opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java # opensearch/src/main/java/org/opensearch/sql/opensearch/storage/scan/AbstractCalciteIndexScan.java
Signed-off-by: Heng Qian <qianheng@amazon.com>
|
The backport to To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.19-dev 2.19-dev
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.19-dev
# Create a new branch
git switch --create backport/backport-3850-to-2.19-dev
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 0b4423e9f3b50670af922a8608116d7182fd728f
# Push it to GitHub
git push --set-upstream origin backport/backport-3850-to-2.19-dev
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.19-devThen, create a pull request where the |
|
@qianheng-aws please manually backport via above instructions. |
* Support partial filter push down Signed-off-by: Heng Qian <qianheng@amazon.com> * Add doc for PushDownAction Signed-off-by: Heng Qian <qianheng@amazon.com> * Fix IT Signed-off-by: Heng Qian <qianheng@amazon.com> * Refine code to only keep non-push-down condition in the new filter Signed-off-by: Heng Qian <qianheng@amazon.com> * Refine code Signed-off-by: Heng Qian <qianheng@amazon.com> * Only show the pushed conditions in the PushDownContext Signed-off-by: Heng Qian <qianheng@amazon.com> * Ignore test when push down disabled Signed-off-by: Heng Qian <qianheng@amazon.com> * Fix IT after merging main Signed-off-by: Heng Qian <qianheng@amazon.com> * Fix IT because mapping changed after merging main Signed-off-by: Heng Qian <qianheng@amazon.com> --------- Signed-off-by: Heng Qian <qianheng@amazon.com> (cherry picked from commit 0b4423e)
Backport PR is here: #3899 |
* Support partial filter push down * Add doc for PushDownAction * Fix IT * Refine code to only keep non-push-down condition in the new filter * Refine code * Only show the pushed conditions in the PushDownContext * Ignore test when push down disabled * Fix IT after merging main * Fix IT because mapping changed after merging main --------- (cherry picked from commit 0b4423e) Signed-off-by: Heng Qian <qianheng@amazon.com>
Description
Support partial filter push down
Related Issues
Resolves #3470
Check List
--signoff.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.