On 2017/05/18 10:49, Amit Langote wrote:
> On 2017/05/18 2:14, Dilip Kumar wrote:
>> On Wed, May 17, 2017 at 7:41 PM, <sveinn.sveins...@gmail.com> wrote:
>>> (gdb) bt
>>> #0 0x000000000061ab1b in list_nth ()
>>> #1 0x00000000005e4081 in ExecLockNonLeafAppendTables ()
>>> #2 0x00000000005f4d52 in ExecInitMergeAppend ()
>>> #3 0x00000000005e0365 in ExecInitNode ()
>>> #4 0x00000000005f35a7 in ExecInitLimit ()
>>> #5 0x00000000005e00f3 in ExecInitNode ()
>>> #6 0x00000000005dd207 in standard_ExecutorStart ()
>>> #7 0x00000000006f96d2 in PortalStart ()
>>> #8 0x00000000006f5c7f in exec_simple_query ()
>>> #9 0x00000000006f6fac in PostgresMain ()
>>> #10 0x0000000000475cdc in ServerLoop ()
>>> #11 0x0000000000692ffa in PostmasterMain ()
>>> #12 0x0000000000476600 in main ()
>
> Thanks for the test case Sveinn and thanks Dilip for analyzing.
>
>> Seems like the issue is that the plans under multiple subroots are
>> pointing to the same partitioned_rels.
>
> That's correct.
>
>> If I am not getting it wrong "set_plan_refs(PlannerInfo *root, Plan
>> *plan, int rtoffset)" the rtoffset is specific to the subroot. Now,
>> problem is that set_plan_refs called for different subroot is updating
>> the same partition_rel info and make this value completely wrong which
>> will ultimately make ExecLockNonLeafAppendTables to access the out of
>> bound "rte" index.
>
> Yes.
>
>> set_plan_refs
>> {
>> [clipped]
>> case T_MergeAppend:
>> {
>> [clipped]
>>
>> foreach(l, splan->partitioned_rels)
>> {
>> lfirst_int(l) += rtoffset;
>>
>>
>> I think the solution should be that create_merge_append_path make the
>> copy of partitioned_rels list?
>
> Yes, partitioned_rels should be copied.
>
>> Attached patch fixes the problem but I am not completely sure about the fix.
>
> Thanks for creating the patch, although I think a better fix would be to
> make get_partitioned_child_rels() do the list_copy. That way, any other
> users of partitioned_rels will not suffer the same issue. Attached patch
> implements that, along with a regression test.
>
> Added to the open items.
Oops, forgot to cc -hackers. Patch attached again.
Thanks,
Amit
>From 73754e24178087cf8aa53904acfb032343be25df Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Thu, 18 May 2017 10:45:44 +0900
Subject: [PATCH] Fix crash when partitioned table Append referenced in
subplans
---
src/backend/optimizer/plan/planner.c | 2 +-
src/test/regress/expected/inherit.out | 31 +++++++++++++++++++++++++++++++
src/test/regress/sql/inherit.sql | 10 ++++++++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index c4a5651abd..bc2797e42c 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -6076,7 +6076,7 @@ get_partitioned_child_rels(PlannerInfo *root, Index rti)
if (pc->parent_relid == rti)
{
- result = pc->child_rels;
+ result = list_copy(pc->child_rels);
break;
}
}
diff --git a/src/test/regress/expected/inherit.out b/src/test/regress/expected/inherit.out
index af7090ba0d..35d182d599 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1918,3 +1918,34 @@ explain (costs off) select * from mcrparted where a = 20 and c > 20; -- scans mc
(7 rows)
drop table mcrparted;
+-- check that partitioned table Appends cope with being referenced in
+-- subplans
+create table parted_minmax (a int, b varchar(16)) partition by range (a);
+create table parted_minmax1 partition of parted_minmax for values from (1) to (10);
+create index parted_minmax1i on parted_minmax1 (a, b);
+insert into parted_minmax values (1,'12345');
+explain (costs off) select min(a), max(a) from parted_minmax where b = '12345';
+ QUERY PLAN
+-------------------------------------------------------------------------------------------------------
+ Result
+ InitPlan 1 (returns $0)
+ -> Limit
+ -> Merge Append
+ Sort Key: parted_minmax1.a
+ -> Index Only Scan using parted_minmax1i on parted_minmax1
+ Index Cond: ((a IS NOT NULL) AND (b = '12345'::text))
+ InitPlan 2 (returns $1)
+ -> Limit
+ -> Merge Append
+ Sort Key: parted_minmax1_1.a DESC
+ -> Index Only Scan Backward using parted_minmax1i on parted_minmax1 parted_minmax1_1
+ Index Cond: ((a IS NOT NULL) AND (b = '12345'::text))
+(13 rows)
+
+select min(a), max(a) from parted_minmax where b = '12345';
+ min | max
+-----+-----
+ 1 | 1
+(1 row)
+
+drop table parted_minmax;
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 7f34f43ec0..70fe971d51 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -661,3 +661,13 @@ explain (costs off) select * from mcrparted where a > -1; -- scans all partition
explain (costs off) select * from mcrparted where a = 20 and abs(b) = 10 and c > 10; -- scans mcrparted4
explain (costs off) select * from mcrparted where a = 20 and c > 20; -- scans mcrparted3, mcrparte4, mcrparte5
drop table mcrparted;
+
+-- check that partitioned table Appends cope with being referenced in
+-- subplans
+create table parted_minmax (a int, b varchar(16)) partition by range (a);
+create table parted_minmax1 partition of parted_minmax for values from (1) to (10);
+create index parted_minmax1i on parted_minmax1 (a, b);
+insert into parted_minmax values (1,'12345');
+explain (costs off) select min(a), max(a) from parted_minmax where b = '12345';
+select min(a), max(a) from parted_minmax where b = '12345';
+drop table parted_minmax;
--
2.11.0
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
