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

Aggregate pushdown for count(const) or count(non null) in JDBC connectors #4362

Closed
sajjoseph opened this issue Jul 6, 2020 · 7 comments · Fixed by #4473
Closed

Aggregate pushdown for count(const) or count(non null) in JDBC connectors #4362

sajjoseph opened this issue Jul 6, 2020 · 7 comments · Fixed by #4473
Milestone

Comments

@sajjoseph
Copy link
Contributor

Aggregate pushdown currently doesn't support the following use case when the count is done on a constant value.

select count(1) from jdbcdb.schema.table;

Current aggregate pushdown implementation generates this query on the datasource side - select 1 from schema.table;.
Instead we like to see - select count(1) from schema.table; or a flavor of it with count operation pushed down.
More details are here - slack link

@Lewuathe
Copy link
Member

Lewuathe commented Jul 9, 2020

We have SimplifyCountOverConstant rule. Does it mean the rule does not work properly? Looks like it supports simplifying the count over constant.

@kokosing
Copy link
Member

kokosing commented Jul 9, 2020

It looks like we run SimplifyCountOverConstant after PushAggregationIntoTableScan.

@Lewuathe
Copy link
Member

@kokosing How can we test the dependency of the rules? We can use BaseRuleTest or RuleTester to verify the rule independently. But I'm not sure how to check the final output of the sequence of multiple rules.

@kokosing
Copy link
Member

I think we need to add this case to io.prestosql.plugin.postgresql.TestPostgreSqlIntegrationSmokeTest#testAggregationPushdown and reorder rules. This will give us good enough coverage.

@Lewuathe
Copy link
Member

Lewuathe commented Jul 14, 2020

@kokosing
I tried to test the following queries. The former was correctly pushdown to the connector but the latter was not.

        assertPushedDown("SELECT count(*) FROM nation");  // Aggregation pushdown happens
        assertPushedDown("SELECT count(1) FROM nation");  // It does not happen

PushAggregationIntoTableScan optimizer was not triggered. It looks like the source of the aggregation node is not TableScanNode, actually ProjectNode. The plan of the latter SQL cannot be applied to the optimizer.

Do you have any idea why aggregation for count(1) is not over table scan? I guess it can be affected by other optimizers but not sure.

@alexjo2144
Copy link
Member

Hey @Lewuathe. @findepi and I were taking a look at this today too. Can you try moving SimplifyCountOverConstant to run before columnPruningOptimizer. That should take care of the extra ProjectNode.

@Lewuathe
Copy link
Member

@alexjo2144 Thanks! I confirmed putting the SimplifyCountOverConstant before column pruning fixed the issue.

@findepi findepi mentioned this issue Jul 23, 2020
8 tasks
@findepi findepi added this to the 340 milestone Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants