Skip to content
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

Push down limit through eval #2876

Merged
merged 5 commits into from
Aug 9, 2024

Conversation

qianheng-aws
Copy link
Contributor

@qianheng-aws qianheng-aws commented Jul 31, 2024

Description

Add a rule EvalPushDown.PUSH_DOWN_LIMIT, this rule will push down limit under eval. Thus, limit has chance to be pushed down into TableScanBuilder later.

e.g.

POST _plugins/_ppl/_explain
{
  "query": """
    source=opensearch_dashboards_sample_data_flights |  eval FlightMin = FlightTimeMin | head 5 | fields  FlightMin
  """
}

# Before optimization
{
  "root": {
    "name": "ProjectOperator",
    "description": {
      "fields": "[FlightMin]"
    },
    "children": [
      {
        "name": "LimitOperator",
        "description": {
          "limit": 5,
          "offset": 0
        },
        "children": [
          {
            "name": "EvalOperator",
            "description": {
              "expressions": {
                "FlightMin": "FlightTimeMin"
              }
            },
            "children": [
              {
                "name": "OpenSearchIndexScan",
                "description": {
                  "request": """OpenSearchQueryRequest(indexName=opensearch_dashboards_sample_data_flights, 

sourceBuilder={"from":0,"size":200,"timeout":"1m"}, searchDone=false)"""
                },
                "children": []
              }
            ]
          }
        ]
      }
    ]
  }
}

# After optimization
{
  "root": {
    "name": "ProjectOperator",
    "description": {
      "fields": "[FlightMin]"
    },
    "children": [
      {
        "name": "EvalOperator",
        "description": {
          "expressions": {
            "FlightMin": "FlightTimeMin"
          }
        },
        "children": [
          {
            "name": "OpenSearchIndexScan",
            "description": {
              "request": """OpenSearchQueryRequest(indexName=opensearch_dashboards_sample_data_flights, 

sourceBuilder={"from":0,"size":5,"timeout":"1m"}, searchDone=false)"""
            },
            "children": []
          }
        ]
      }
    ]
  }
}

Note: Also added TODOs in this PR to implement rules PUSH_DOWN_SORT and PUSH_DOWN_PROJECT as follow-ups, which are more complex due to expression replacement.

Related Issues

Resolves #2903

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • New functionality has javadoc added.
  • New functionality has a user manual doc added.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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.

Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
Copy link

codecov bot commented Jul 31, 2024

Codecov Report

Attention: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 94.62%. Comparing base (593ffab) to head (b7c4129).
Report is 6 commits behind head on main.

Files Patch % Lines
...search/sql/planner/optimizer/pattern/Patterns.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main    #2876   +/-   ##
=========================================
  Coverage     94.62%   94.62%           
- Complexity     5166     5170    +4     
=========================================
  Files           508      509    +1     
  Lines         14539    14559   +20     
  Branches        959      959           
=========================================
+ Hits          13758    13777   +19     
- Misses          740      741    +1     
  Partials         41       41           
Flag Coverage Δ
sql-engine 94.62% <95.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Heng Qian <qianheng@amazon.com>
dai-chen
dai-chen previously approved these changes Aug 5, 2024
Copy link
Collaborator

@dai-chen dai-chen left a comment

Choose a reason for hiding this comment

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

Minor comment. Thanks for the changes!

Signed-off-by: Heng Qian <qianheng@amazon.com>
Signed-off-by: Heng Qian <qianheng@amazon.com>
@dai-chen dai-chen added the enhancement New feature or request label Aug 7, 2024
@LantaoJin
Copy link
Member

@qianheng-aws could you update the PR title more precise? PR title will be recorded in release notes.

Copy link
Member

@LantaoJin LantaoJin left a comment

Choose a reason for hiding this comment

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

LGTM

@qianheng-aws qianheng-aws changed the title Add EvalPushDown rule in LogicalPlanOptimizer Add rule EvalPushDown.PUSH_DOWN_LIMIT in LogicalPlanOptimizer Aug 9, 2024
@qianheng-aws qianheng-aws changed the title Add rule EvalPushDown.PUSH_DOWN_LIMIT in LogicalPlanOptimizer Push down limit through eval Aug 9, 2024
@LantaoJin LantaoJin merged commit 4a735ea into opensearch-project:main Aug 9, 2024
14 of 15 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 9, 2024
(cherry picked from commit 4a735ea)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@qianheng-aws qianheng-aws deleted the EvalPushDown branch August 12, 2024 03:13
manasvinibs pushed a commit to manasvinibs/sql that referenced this pull request Aug 14, 2024
jzonthemtn pushed a commit to jzonthemtn/sql that referenced this pull request Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

push down limit
3 participants