On 2018/08/09 0:48, Tom Lane wrote: > David Rowley <david.row...@2ndquadrant.com> writes: >> On 8 August 2018 at 17:28, Amit Langote <langote_amit...@lab.ntt.co.jp> >> wrote: >>> Attached is a patch which modifies the if test to compare relids instead >>> of RelOptInfo pointers. > >> Thanks for investigating and writing a patch. I agree with the fix. > > I changed this to compare the relid sets not just rel->relid, since > rel->relid is only reliable for baserels. The partitioned rel could > safely be assumed to be a baserel, but I'm less comfortable with > supposing that the parentrel always will be. Otherwise, added a > test case based on Rushabh's example and pushed. (I'm not quite > sure if the plan will be stable enough to satisfy the buildfarm, > but we'll soon find out ...)
Thank you for committing, agreed that comparing relid sets for equality might be more future-proof. About the test case, wondering if we should, like David seemed to suggest, add a test case that would actually use run-time pruning? Maybe, even better if the new test also had partitioned parent under UNION ALL parent under ModifyTable. Something like in the attached? One reason why we should adapt such a test case is that, in the future, we may arrange for make_partitionedrel_pruneinfo(), whose code we just fixed, to not be called if we know that run-time pruning is not needed. It seems that that's true for the test added by the commit, that is, it doesn't need run-time pruning. Regards, Amit
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out index 693c348185..61457862a9 100644 --- a/src/test/regress/expected/partition_prune.out +++ b/src/test/regress/expected/partition_prune.out @@ -2478,6 +2478,8 @@ deallocate ab_q3; deallocate ab_q4; deallocate ab_q5; -- UPDATE on a partition subtree has been seen to have problems. +set enable_hashjoin to off; +set enable_mergejoin to off; insert into ab values (1,2); explain (analyze, costs off, summary off, timing off) update ab_a1 set b = 3 from ab where ab.a = 1 and ab.a = ab_a1.a; @@ -2556,6 +2558,69 @@ table ab; 1 | 3 (1 row) +truncate ab; +insert into ab values (1, 1), (1, 2), (1, 3); +explain (analyze, costs off, summary off, timing off) +update ab_a1 set b = 3 from (select * from (select * from ab_a2 union all select 1, 2) s where s.b = (select 2)) ss where ss.a = ab_a1.a; + QUERY PLAN +--------------------------------------------------------------------------------- + Update on ab_a1 (actual rows=0 loops=1) + Update on ab_a1_b1 + Update on ab_a1_b2 + Update on ab_a1_b3 + InitPlan 1 (returns $0) + -> Result (actual rows=1 loops=1) + -> Nested Loop (actual rows=1 loops=1) + -> Append (actual rows=1 loops=1) + -> Seq Scan on ab_a2_b1 (never executed) + Filter: (b = $0) + -> Seq Scan on ab_a2_b2 (actual rows=0 loops=1) + Filter: (b = $0) + -> Seq Scan on ab_a2_b3 (never executed) + Filter: (b = $0) + -> Subquery Scan on "*SELECT* 2" (actual rows=1 loops=1) + -> Result (actual rows=1 loops=1) + One-Time Filter: (2 = $0) + -> Index Scan using ab_a1_b1_a_idx on ab_a1_b1 (actual rows=1 loops=1) + Index Cond: (a = ab_a2_b1.a) + -> Nested Loop (actual rows=1 loops=1) + -> Append (actual rows=1 loops=1) + -> Seq Scan on ab_a2_b1 (never executed) + Filter: (b = $0) + -> Seq Scan on ab_a2_b2 (actual rows=0 loops=1) + Filter: (b = $0) + -> Seq Scan on ab_a2_b3 (never executed) + Filter: (b = $0) + -> Subquery Scan on "*SELECT* 2_1" (actual rows=1 loops=1) + -> Result (actual rows=1 loops=1) + One-Time Filter: (2 = $0) + -> Index Scan using ab_a1_b2_a_idx on ab_a1_b2 (actual rows=1 loops=1) + Index Cond: (a = ab_a2_b1.a) + -> Nested Loop (actual rows=1 loops=1) + -> Append (actual rows=1 loops=1) + -> Seq Scan on ab_a2_b1 (never executed) + Filter: (b = $0) + -> Seq Scan on ab_a2_b2 (actual rows=0 loops=1) + Filter: (b = $0) + -> Seq Scan on ab_a2_b3 (never executed) + Filter: (b = $0) + -> Subquery Scan on "*SELECT* 2_2" (actual rows=1 loops=1) + -> Result (actual rows=1 loops=1) + One-Time Filter: (2 = $0) + -> Index Scan using ab_a1_b3_a_idx on ab_a1_b3 (actual rows=1 loops=1) + Index Cond: (a = ab_a2_b1.a) +(45 rows) + +table ab; + a | b +---+--- + 1 | 3 + 1 | 3 + 1 | 3 +(3 rows) + +reset enable_hashjoin; +reset enable_mergejoin; drop table ab, lprt_a; -- Join create table tbl1(col1 int); diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql index 935c509b29..e78220a3e5 100644 --- a/src/test/regress/sql/partition_prune.sql +++ b/src/test/regress/sql/partition_prune.sql @@ -555,10 +555,19 @@ deallocate ab_q4; deallocate ab_q5; -- UPDATE on a partition subtree has been seen to have problems. +set enable_hashjoin to off; +set enable_mergejoin to off; insert into ab values (1,2); explain (analyze, costs off, summary off, timing off) update ab_a1 set b = 3 from ab where ab.a = 1 and ab.a = ab_a1.a; table ab; +truncate ab; +insert into ab values (1, 1), (1, 2), (1, 3); +explain (analyze, costs off, summary off, timing off) +update ab_a1 set b = 3 from (select * from (select * from ab_a2 union all select 1, 2) s where s.b = (select 2)) ss where ss.a = ab_a1.a; +table ab; +reset enable_hashjoin; +reset enable_mergejoin; drop table ab, lprt_a;
