trinodb / trino

Official repository of Trino, the distributed SQL query engine for big data, formerly known as PrestoSQL (https://trino.io)
https://trino.io
Apache License 2.0
10.25k stars 2.95k forks source link

Support pushing down grouping keys with function #9067

Open jerryleooo opened 3 years ago

jerryleooo commented 3 years ago

These tests are done on PostgreSQL connector with TPC-H data.

Trino query: SELECT lower(o_clerk), SUM(o_totalprice) FROM orders GROUP BY LOWER(o_clerk); Generated PostgreSQL query: SELECT "o_totalprice", "o_clerk" FROM "public"."orders" (full table scan)

Trino query: SELECT o_clerk, SUM(o_totalprice) FROM orders GROUP BY o_clerk; Generated PostgreSQL query: SELECT "o_clerk", "_pfgnrtd_0" FROM (SELECT "o_clerk", sum("o_totalprice") AS "_pfgnrtd_0" FROM "public"."orders" GROUP BY "o_clerk") o (aggregation can be pushed down)

Not sure if this belongs to https://github.com/trinodb/trino/pull/7994

assaf2 commented 3 years ago

lower(o_clerk) projection will be pushed down after #7994

jerryleooo commented 3 years ago

Hi @assaf2 thanks for the hard work! I am reviewing #7994 but seems the projection is not pushed down for PostgreSQL connector, I guess there is some work that will be done in another PR?

Changes I tried: https://gist.github.com/jerryleooo/b2b92c2ce04a8ef78bfbb7e7ddc9a8f2

assaf2 commented 3 years ago

When the rule PushProjectionIntoTableScan calls metadata.applyProjection(), the argument List<ConnectorExpression> projections should contain a io.trino.spi.expression.Call. For example, I've changed the first query of io.trino.plugin.postgresql.TestPostgreSqlConnectorTest#testPredicatePushdown to SELECT regionkey, nationkey, lower(name) FROM nation WHERE name = #'ROMANIA' and got the following: projections = {RegularImmutableList@21458} size = 3 0 = {Variable@21464} "nationkey::bigint" 1 = {Variable@21465} "regionkey::bigint" 2 = {Call@21466} "Call{schemaCatalogName=Optional.empty, name='lower', arguments=[name::varchar(25)]} ".

slhmy commented 2 years ago

Hi @assaf2 thanks for the hard work! I am reviewing #7994 but seems the projection is not pushed down for PostgreSQL connector, I guess there is some work that will be done in another PR?

Changes I tried: https://gist.github.com/jerryleooo/b2b92c2ce04a8ef78bfbb7e7ddc9a8f2

Hi, @jerryleooo. I'm trying similar things with PostgreSQL connector. I pushed Like expression(which appears in the filter) down. And this cause myself changing applyFilter method in DefaultJdbcMetadata.java. In this changes, I think we also missed something to be changed in DefaultJdbcMetadata.java. Maybe something in applyProjection.

Specifically we can follow implementation in applyAggregation, I think it's similar with aggregation pushdown.