Presto: Executing PickTableLayout before AddExchanges breaks index joins
In particular commit e6d963b1b3 causes index joins to fail with "Tuple domain handles must have assigned symbols".
nezihyigitbasi
All 17 comments
2018-01-24 05:38:51 INFO TIMELINE: Query 20180123_235350_00000_q46r4 :: Transaction:[abb9d84a-2ff9-4c4f-9ece-118620fa2a11] :: elapsed 372ms :: planning 353ms :: scheduling 19ms :: running 0ms :: finishing 19ms :: begin 2018-01-24T05:38:50.555+05:45 :: end 2018-01-24T05:38:50.927+05:45
java.lang.AssertionError: Execution of 'actual' query failed: SELECT COUNT(*) FROM lineitem JOIN orders ON lineitem.orderkey = orders.orderkey AND orders.custkey = 1
at org.testng.Assert.fail(Assert.java:83)
at com.facebook.presto.tests.QueryAssertions.assertQuery(QueryAssertions.java:87)
at com.facebook.presto.tests.AbstractTestQueryFramework.assertQuery(AbstractTestQueryFramework.java:119)
at com.facebook.presto.tests.AbstractTestQueryFramework.assertQuery(AbstractTestQueryFramework.java:114)
at com.facebook.presto.tests.AbstractTestQueries.testJoinWithNonJoinExpression(AbstractTestQueries.java:3091)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:104)
at org.testng.internal.Invoker.invokeMethod(Invoker.java:645)
at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:851)
at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1177)
at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
at org.testng.TestRunner.privateRun(TestRunner.java:756)
at org.testng.TestRunner.run(TestRunner.java:610)
at org.testng.SuiteRunner.runTest(SuiteRunner.java:387)
at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:382)
at org.testng.SuiteRunner.privateRun(SuiteRunner.java:340)
at org.testng.SuiteRunner.run(SuiteRunner.java:289)
at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
at org.testng.TestNG.runSuitesSequentially(TestNG.java:1293)
at org.testng.TestNG.runSuitesLocally(TestNG.java:1218)
at org.testng.TestNG.runSuites(TestNG.java:1133)
at org.testng.TestNG.run(TestNG.java:1104)
at org.testng.IDEARemoteTestNG.run(IDEARemoteTestNG.java:72)
at org.testng.RemoteTestNGStarter.main(RemoteTestNGStarter.java:123)
Caused by: java.lang.IllegalArgumentException: Tuple domain handles must have assigned symbols
at com.google.common.base.Preconditions.checkArgument(Preconditions.java:122)
at com.facebook.presto.sql.planner.plan.IndexSourceNode.lambda$new$0(IndexSourceNode.java:72)
at java.util.Optional.ifPresent(Optional.java:159)
at com.facebook.presto.sql.planner.plan.IndexSourceNode.<init>(IndexSourceNode.java:71)
at com.facebook.presto.sql.planner.optimizations.IndexJoinOptimizer$IndexSourceRewriter.planTableScan(IndexJoinOptimizer.java:304)
at com.facebook.presto.sql.planner.optimizations.IndexJoinOptimizer$IndexSourceRewriter.visitTableScan(IndexJoinOptimizer.java:265)
at com.facebook.presto.sql.planner.optimizations.IndexJoinOptimizer$IndexSourceRewriter.visitTableScan(IndexJoinOptimizer.java:222)
at com.facebook.presto.sql.planner.plan.TableScanNode.accept(TableScanNode.java:136)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter.rewriteWith(SimplePlanRewriter.java:32)
at com.facebook.presto.sql.planner.optimizations.IndexJoinOptimizer$IndexSourceRewriter.rewriteWithIndex(IndexJoinOptimizer.java:248)
at com.facebook.presto.sql.planner.optimizations.IndexJoinOptimizer$Rewriter.visitJoin(IndexJoinOptimizer.java:128)
at com.facebook.presto.sql.planner.optimizations.IndexJoinOptimizer$Rewriter.visitJoin(IndexJoinOptimizer.java:89)
at com.facebook.presto.sql.planner.plan.JoinNode.accept(JoinNode.java:220)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter$RewriteContext.rewrite(SimplePlanRewriter.java:84)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter$RewriteContext.lambda$defaultRewrite$0(SimplePlanRewriter.java:73)
at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
at java.util.Collections$2.tryAdvance(Collections.java:4717)
at java.util.Collections$2.forEachRemaining(Collections.java:4725)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter$RewriteContext.defaultRewrite(SimplePlanRewriter.java:74)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter.visitPlan(SimplePlanRewriter.java:38)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter.visitPlan(SimplePlanRewriter.java:22)
at com.facebook.presto.sql.planner.plan.PlanVisitor.visitAggregation(PlanVisitor.java:29)
at com.facebook.presto.sql.planner.plan.AggregationNode.accept(AggregationNode.java:182)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter$RewriteContext.rewrite(SimplePlanRewriter.java:84)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter$RewriteContext.lambda$defaultRewrite$0(SimplePlanRewriter.java:73)
at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
at java.util.Collections$2.tryAdvance(Collections.java:4717)
at java.util.Collections$2.forEachRemaining(Collections.java:4725)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter$RewriteContext.defaultRewrite(SimplePlanRewriter.java:74)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter.visitPlan(SimplePlanRewriter.java:38)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter.visitPlan(SimplePlanRewriter.java:22)
at com.facebook.presto.sql.planner.plan.PlanVisitor.visitOutput(PlanVisitor.java:49)
at com.facebook.presto.sql.planner.plan.OutputNode.accept(OutputNode.java:82)
at com.facebook.presto.sql.planner.plan.SimplePlanRewriter.rewriteWith(SimplePlanRewriter.java:32)
at com.facebook.presto.sql.planner.optimizations.IndexJoinOptimizer.optimize(IndexJoinOptimizer.java:86)
at com.facebook.presto.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:133)
at com.facebook.presto.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:122)
at com.facebook.presto.execution.SqlQueryExecution.doAnalyzeQuery(SqlQueryExecution.java:368)
at com.facebook.presto.execution.SqlQueryExecution.analyzeQuery(SqlQueryExecution.java:347)
at com.facebook.presto.execution.SqlQueryExecution.start(SqlQueryExecution.java:279)
at com.facebook.presto.execution.QueuedExecution.lambda$start$1(QueuedExecution.java:62)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
at com.facebook.presto.tests.AbstractTestQueries.testJoinWithNonJoinExpression(AbstractTestQueries.java:3091)
i see this is an existing test. Which one?
Also, i added the below test to AbstractTestIndexedQueries, but I couldn't repro this exception (i got different one, unrelated, fix: https://github.com/prestodb/presto/pull/9806)
@Test
public void testJoinWithNonJoinExpression()
{
assertQuery("SELECT COUNT(*) FROM lineitem JOIN orders ON lineitem.orderkey = orders.orderkey AND orders.custkey = 1");
}
findepi
on 24 Jan 2018
@nezihyigitbasi I posted possible fix: https://github.com/prestodb/presto/pull/9808. My suspicion is that connector returned a layout with predicate which contains columns not accessible for table scan (absent in TableScan#assignments). So far there was no check for that.
Before #9701, neither IndexJoinOptimizer or AddExchanges has not seen real predicate in TableScan#currentConstraint. So possibly there are more bugs.. ; /
Can you please check if attached PR fixes your problem?
I think that in order to reproduce this issue we might want change the indexed tpch connector. Maybe by solving TODO from https://github.com/prestodb/presto/blob/master/presto-tpch/src/main/java/com/facebook/presto/tpch/TpchMetadata.java#L206.
kokosing
on 24 Jan 2018
I think that in order to reproduce this issue we might want change the indexed tpch connector. Maybe by solving TODO from https://github.com/prestodb/presto/blob/master/presto-tpch/src/main/java/com/facebook/presto/tpch/TpchMetadata.java#L206.
We might want to change (break) com.facebook.presto.tests.tpch.TpchIndexMetadata#resolveIndex as this method filters out non indexable columns (output of table scan) from tuple domain.
kokosing
on 24 Jan 2018
It looks like I was correct or there is another case which produces similar stack trace. I was able to reproduce the bug, see https://github.com/prestodb/presto/pull/9810. However I am not sure if changes in TpchIndexMetadata are desirable.
Locally #9808 fixed issue exposed by #9810.
kokosing
on 24 Jan 2018
Locally #9808 fixed issue exposed by #9810.
It also required #9806.
kokosing
on 24 Jan 2018
Apparently #9808 it is not as easy as I expected. I had to fix some test failures, which required to filter TableScanNode#currentConstraint in PruneUnreferencedOutputs and AddExchanges. However I then got some mysterious error when duing DELETE on partitioned hive tables.
kokosing
on 24 Jan 2018
9813 is much simpler than #9808 and maybe it could get in as a bugfix, then I will try to fix #9808 (after the release).
kokosing
on 24 Jan 2018
We get this exception for an internal connector and I couldn't find a way to repro it yet.
nezihyigitbasi
on 24 Jan 2018
Would it be possible to check if @kokosing change fixes the problem?
findepi
on 24 Jan 2018
Tried #9813 and the test failed with a different issue, the expected output didn't match the actual output:
java.lang.AssertionError: For query:
SELECT COUNT(*) FROM lineitem JOIN orders ON lineitem.orderkey = orders.orderkey AND orders.custkey = 1
not equal
Actual 1 rows:
[60175]
Expected 1 rows:
[35]
As another data point if I move PickTableLayout right after PruneUnreferencedOutputs and right before IndexJoinOptimizer it works fine:
// this is the working order in PlanOptimizers
new PruneUnreferencedOutputs(), // Make sure to run this before index join. Filtered projections may not have all the columns.
new IterativeOptimizer(
stats,
new PickTableLayout(metadata).rules()),
new IndexJoinOptimizer(metadata), // Run this after projections and filters have been fully simplified and pushed down
@nezihyigitbasi One last try: https://github.com/prestodb/presto/pull/9815. The difference is that you cannot simply filter columns used in currentConstraint. For example FilterNode predicate could be fully pushed down to TableScanNode#currentConstraint and then columns used by the above FilterNode might be removed from output from table scan.
Can you please give a shot and see if it helps?
kokosing
on 24 Jan 2018
@martint and I debugged the problem and @martint has a fix that he is testing now.
nezihyigitbasi
on 24 Jan 2018
Anyway, can please you give it a try if it is not a problem for you. I wonder if I was correct.
kokosing
on 24 Jan 2018
@kokosing the particular test that was failing works with this patch too.
nezihyigitbasi
on 24 Jan 2018
Great. Thank you.
kokosing
on 24 Jan 2018
Related issues
aminalaee
路
3Comments
rajeshd3v
路
3Comments
tomz
路
3Comments
yaxxie
路
4Comments
electrum
路
4Comments
Most helpful comment
@kokosing the particular test that was failing works with this patch too.