Open takezoe opened 2 years ago
@martint Hi, could you take a look at this since you are the author of https://github.com/trinodb/trino/pull/1550?
I tried removing the scope check which was added in #1550 however the following test case failed with Division by zero
which is the same error on 319.
https://github.com/trinodb/trino/blob/ae713b7699e807eea3fccaa56abaf4cc8e393ccc/core/trino-main/src/test/java/io/trino/sql/query/TestPredicatePushdown.java#L45-L62
Preventing constants pushdown seems to be beneficial in this case. However, I feel that the downsides by it outweighs the benefits...
Asking on Slack: https://trinodb.slack.com/archives/CGB0QHWSW/p1645535658457569
Since 320, predicate pushdown does't work for constants from outside the scope of subquery and it slowdowns specific type of queries significantly.
For example, some queries have a chance to reduce the number of records in early stages by pushdown constants and it greatly helps the join performance in later stages. This type of queries got very slow or even suffered from the memory exceeding error on 320 or later.
A simple query to reproduce:
Explain on 319
Explain on the latest master:
I confirmed that the following scope check introduced in 320 prevents the pushdown, but not sure if just removing this scope check is fine. https://github.com/trinodb/trino/pull/1550/files#diff-d5f6d85a278c69d6969856f07362e37d18eb2fe6fd7c79526f035c3e9b4cfca0R323-R333