Skip to content

Commit 8e34250

Browse files
committed
Fix cte filter pushdown wrong results by splitting SpecialFormExpressions
1 parent 6c2f50d commit 8e34250

File tree

2 files changed

+78
-5
lines changed

2 files changed

+78
-5
lines changed

presto-hive/src/test/java/com/facebook/presto/hive/TestCteExecution.java

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,72 @@ public void testPersistentCteWithTimeStampWithTimeZoneType()
113113
testQuery));
114114
}
115115

116+
@Test
117+
public void testComplexCommonFilterPushdown()
118+
{
119+
QueryRunner queryRunner = getQueryRunner();
120+
String testQuery = "WITH order_platform_data AS (\n" +
121+
" SELECT\n" +
122+
" o.orderkey AS order_key,\n" +
123+
" o.orderdate AS datestr,\n" +
124+
" o.orderpriority AS event_type\n" +
125+
" FROM\n" +
126+
" orders o\n" +
127+
" WHERE\n" +
128+
" o.orderdate BETWEEN DATE '1995-01-01' AND DATE '1995-01-31'\n" +
129+
" AND o.orderpriority IN ('1-URGENT', '3-MEDIUM')\n" +
130+
" UNION ALL\n" +
131+
" SELECT\n" +
132+
" l.orderkey AS order_key,\n" +
133+
" o.orderdate AS datestr,\n" +
134+
" o.orderpriority AS event_type\n" +
135+
" FROM\n" +
136+
" lineitem l\n" +
137+
" JOIN orders o ON l.orderkey = o.orderkey\n" +
138+
" WHERE\n" +
139+
" o.orderdate BETWEEN DATE '1995-01-01' AND DATE '1995-01-31'\n" +
140+
" AND o.orderpriority IN ('2-HIGH', '5-LOW')\n" +
141+
"),\n" +
142+
"urgent AS (\n" +
143+
" SELECT order_key, datestr\n" +
144+
" FROM order_platform_data\n" +
145+
" WHERE event_type = '1-URGENT'\n" +
146+
"),\n" +
147+
"medium AS (\n" +
148+
" SELECT order_key, datestr\n" +
149+
" FROM order_platform_data\n" +
150+
" WHERE event_type = '3-MEDIUM'\n" +
151+
"),\n" +
152+
"high AS (\n" +
153+
" SELECT order_key, datestr\n" +
154+
" FROM order_platform_data\n" +
155+
" WHERE event_type = '2-HIGH'\n" +
156+
"),\n" +
157+
"low AS (\n" +
158+
" SELECT order_key, datestr\n" +
159+
" FROM order_platform_data\n" +
160+
" WHERE event_type = '5-LOW'\n" +
161+
")\n" +
162+
"SELECT\n" +
163+
" ofin.order_key AS order_key,\n" +
164+
" ofin.datestr AS order_date\n" +
165+
" FROM " +
166+
" urgent ofin\n" +
167+
" LEFT JOIN medium oproc ON ofin.datestr = oproc.datestr\n" +
168+
" LEFT JOIN low on oproc.datestr = low.datestr" +
169+
" LEFT JOIN high on low.datestr = high.datestr" +
170+
" ORDER BY\n" +
171+
" ofin.order_key\n";
172+
compareResults(queryRunner.execute(Session.builder(super.getSession())
173+
.setSystemProperty(PUSHDOWN_SUBFIELDS_ENABLED, "true")
174+
.setSystemProperty(CTE_MATERIALIZATION_STRATEGY, "HEURISTIC_COMPLEX_QUERIES_ONLY")
175+
.setSystemProperty(CTE_FILTER_AND_PROJECTION_PUSHDOWN_ENABLED, "true")
176+
.build(),
177+
testQuery),
178+
queryRunner.execute(getSession(),
179+
testQuery));
180+
}
181+
116182
@Test
117183
public void testPersistentCteWithStructTypes()
118184
{

presto-main/src/main/java/com/facebook/presto/sql/planner/optimizations/CteProjectionAndPredicatePushDown.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import com.facebook.presto.sql.planner.SimplePlanVisitor;
3232
import com.facebook.presto.sql.planner.TypeProvider;
3333
import com.facebook.presto.sql.planner.VariablesExtractor;
34+
import com.facebook.presto.sql.planner.iterative.rule.SimplifyRowExpressions;
3435
import com.facebook.presto.sql.planner.plan.SequenceNode;
3536
import com.facebook.presto.sql.planner.plan.SimplePlanRewriter;
3637
import com.google.common.collect.ImmutableList;
@@ -48,7 +49,6 @@
4849
import static com.facebook.presto.SystemSessionProperties.getCteFilterAndProjectionPushdownEnabled;
4950
import static com.facebook.presto.SystemSessionProperties.isCteMaterializationApplicable;
5051
import static com.facebook.presto.common.type.BooleanType.BOOLEAN;
51-
import static com.facebook.presto.spi.relation.SpecialFormExpression.Form.OR;
5252
import static com.facebook.presto.sql.planner.PlannerUtils.isConstant;
5353
import static com.facebook.presto.sql.planner.plan.ChildReplacer.replaceChildren;
5454
import static com.facebook.presto.sql.relational.Expressions.constant;
@@ -370,14 +370,21 @@ private PlanNode addFilter(PlanNode node, List<RowExpression> predicates)
370370
return node;
371371
}
372372

373-
RowExpression predicate;
373+
RowExpression resultPredicate;
374374
if (predicates.size() == 1) {
375-
predicate = predicates.get(0);
375+
resultPredicate = predicates.get(0);
376376
}
377377
else {
378-
predicate = new SpecialFormExpression(OR, BOOLEAN, predicates);
378+
resultPredicate = predicates.get(0);
379+
for (int i = 1; i < predicates.size(); i++) {
380+
resultPredicate = new SpecialFormExpression(
381+
SpecialFormExpression.Form.OR,
382+
BOOLEAN,
383+
resultPredicate, predicates.get(i));
384+
}
379385
}
380-
return new FilterNode(node.getSourceLocation(), idAllocator.getNextId(), node, predicate);
386+
resultPredicate = SimplifyRowExpressions.rewrite(resultPredicate, metadata, session.toConnectorSession());
387+
return new FilterNode(node.getSourceLocation(), idAllocator.getNextId(), node, resultPredicate);
381388
}
382389

383390
private boolean isConstTrue(List<RowExpression> predicates)

0 commit comments

Comments
 (0)