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

SQL: Pushdown WHERE clause inside subqueries #71362

Merged
merged 5 commits into from
Apr 7, 2021

Conversation

costin
Copy link
Member

@costin costin commented Apr 6, 2021

Push down filters inside subqueries, even when dealing with aggregates.
The rule already existed however it was not being used inside SQL.
When dealing with Aggregates, keep the aggregate functions in place but
try and push down conjunctions on non-aggregates.

@costin costin requested review from astefan, bpintea and matriv April 6, 2021 17:27
@elasticmachine elasticmachine added the Team:QL (Deprecated) Meta label for query languages team label Apr 6, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-ql (Team:QL)

new SimplifyComparisonsArithmetics(DataTypes::areCompatible),
// prune/elimination
new PruneFilters(),
new PruneLiteralsInOrderBy(),
new PruneCast(),
new CombineLimits());
new CombineLimits(),
new PushDownAndCombineFilters()
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved the rule here to be consistent across EQL and SQL.

@@ -189,25 +190,6 @@ protected LogicalPlan rule(Filter filter) {
}
}

static class PushDownAndCombineFilters extends OptimizerRule<Filter> {
Copy link
Member Author

Choose a reason for hiding this comment

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

Extracted this rule from EQL inside QL project.

@costin costin force-pushed the sql/subselect-where-clause branch from 3c582a5 to c161ba1 Compare April 6, 2021 17:29
Push down filters inside subqueries, even when dealing with aggregates.
The rule already existed however it was not being used inside SQL.
When dealing with Aggregates, keep the aggregate functions in place but
try and push down conjunctions on non-aggregates.
@costin costin force-pushed the sql/subselect-where-clause branch from c161ba1 to 9a6d322 Compare April 6, 2021 17:30
Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

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

Looks good. Another very nice improvement for our subquery support!
I'd like to ask a few more tests with more complex expressions, e.g.:

group by with function:

SELECT * FROM (
    SELECT languages + 1 AS lan, MAX(salary) AS max FROM test_emp WHERE languages > 1 GROUP BY lan ORDER BY 1
)
WHERE (lan - 1) > 2

aggregate with function:

SELECT * FROM (
    SELECT gender, MAX(salary) / 100 AS max FROM test_emp WHERE languages > 1 GROUP BY gender ORDER BY gender
) 
WHERE max > 745 AND gender IS NOT NULL

and a combination where we have both complex agg and group by:

SELECT * FROM (
  SELECT lan - 1 AS lan, max / 2 AS max FROM (
    SELECT languages + 1 AS lan, MAX(salary) / 10 AS max FROM test_emp WHERE languages > 1 GROUP BY lan ORDER BY 1
  )
)
WHERE (lan - 1) > 1 AND max - 1000 > 2500

@costin
Copy link
Member Author

costin commented Apr 7, 2021

The last query isn't supported at the moment, see #71350

The query since it's a conjunction and thus the grouping expression is pushed down.

Copy link
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!!

Copy link
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

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

LGTM

@costin costin merged commit 0424b20 into elastic:master Apr 7, 2021
@costin costin deleted the sql/subselect-where-clause branch April 7, 2021 12:06
costin added a commit that referenced this pull request Apr 7, 2021
Push down filters inside subqueries, even when dealing with aggregates.
The rule already existed however it was not being used inside SQL.
When dealing with Aggregates, keep the aggregate functions in place but
try and push down conjunctions on non-aggregates.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants