Amit Langote <[email protected]> writes:
> On 2019/03/30 0:29, Tom Lane wrote:
>> That seems like probably an independent patch --- do you want to write it?
> Here is that patch.
> It revises get_relation_constraints() such that the partition constraint
> is loaded in only the intended cases.
So I see the problem you're trying to solve here, but I don't like this
patch a bit, because it depends on root->inhTargetKind which IMO is a
broken bit of junk that we need to get rid of. Here is an example of
why, with this patch applied:
regression=# create table p (a int) partition by list (a);
CREATE TABLE
regression=# create table p1 partition of p for values in (1);
CREATE TABLE
regression=# set constraint_exclusion to on;
SET
regression=# explain select * from p1 where a = 2;
QUERY PLAN
------------------------------------------
Result (cost=0.00..0.00 rows=0 width=0)
One-Time Filter: false
(2 rows)
So far so good, but watch what happens when we include the same case
in an UPDATE on some other partitioned table:
regression=# create table prtab (a int, b int) partition by list (a);
CREATE TABLE
regression=# create table prtab2 partition of prtab for values in (2);
CREATE TABLE
regression=# explain update prtab set b=b+1 from p1 where prtab.a=p1.a and
p1.a=2;
QUERY PLAN
---------------------------------------------------------------------------
Update on prtab (cost=0.00..82.30 rows=143 width=20)
Update on prtab2
-> Nested Loop (cost=0.00..82.30 rows=143 width=20)
-> Seq Scan on p1 (cost=0.00..41.88 rows=13 width=10)
Filter: (a = 2)
-> Materialize (cost=0.00..38.30 rows=11 width=14)
-> Seq Scan on prtab2 (cost=0.00..38.25 rows=11 width=14)
Filter: (a = 2)
(8 rows)
No constraint exclusion, while in v10 you get
Update on prtab (cost=0.00..0.00 rows=0 width=0)
-> Result (cost=0.00..0.00 rows=0 width=0)
One-Time Filter: false
The reason is that this logic supposes that root->inhTargetKind describes
*all* partitioned tables in the query, which is obviously wrong.
Now maybe we could make it work by doing something like
if (rel->reloptkind == RELOPT_BASEREL &&
(root->inhTargetKind == INHKIND_NONE ||
rel->relid != root->parse->resultRelation))
but I find that pretty messy, plus it's violating the concept that we
shouldn't be allowing messiness from inheritance_planner to leak into
other places. What I'd rather do is have this test just read
if (rel->reloptkind == RELOPT_BASEREL)
Making it be that way causes some changes in the partition_prune results,
as attached, which suggest that removing the enable_partition_pruning
check as you did wasn't such a great idea either. However, if I add
that back in, then it breaks the proposed new regression test case.
I'm not at all clear on what we think the interaction between
enable_partition_pruning and constraint_exclusion ought to be,
so I'm not sure what the appropriate resolution is here. Thoughts?
BTW, just about all the other uses of root->inhTargetKind seem equally
broken from here; none of them are accounting for whether the rel in
question is the query target.
regards, tom lane
diff -U3 /home/postgres/pgsql/src/test/regress/expected/partition_prune.out /home/postgres/pgsql/src/test/regress/results/partition_prune.out
--- /home/postgres/pgsql/src/test/regress/expected/partition_prune.out 2019-04-01 12:39:52.613109088 -0400
+++ /home/postgres/pgsql/src/test/regress/results/partition_prune.out 2019-04-01 13:18:02.852615395 -0400
@@ -3409,24 +3409,18 @@
--------------------------
Update on pp_lp
Update on pp_lp1
- Update on pp_lp2
-> Seq Scan on pp_lp1
Filter: (a = 1)
- -> Seq Scan on pp_lp2
- Filter: (a = 1)
-(7 rows)
+(4 rows)
explain (costs off) delete from pp_lp where a = 1;
QUERY PLAN
--------------------------
Delete on pp_lp
Delete on pp_lp1
- Delete on pp_lp2
-> Seq Scan on pp_lp1
Filter: (a = 1)
- -> Seq Scan on pp_lp2
- Filter: (a = 1)
-(7 rows)
+(4 rows)
set constraint_exclusion = 'off'; -- this should not affect the result.
explain (costs off) select * from pp_lp where a = 1;
@@ -3444,24 +3438,18 @@
--------------------------
Update on pp_lp
Update on pp_lp1
- Update on pp_lp2
-> Seq Scan on pp_lp1
Filter: (a = 1)
- -> Seq Scan on pp_lp2
- Filter: (a = 1)
-(7 rows)
+(4 rows)
explain (costs off) delete from pp_lp where a = 1;
QUERY PLAN
--------------------------
Delete on pp_lp
Delete on pp_lp1
- Delete on pp_lp2
-> Seq Scan on pp_lp1
Filter: (a = 1)
- -> Seq Scan on pp_lp2
- Filter: (a = 1)
-(7 rows)
+(4 rows)
drop table pp_lp;
-- Ensure enable_partition_prune does not affect non-partitioned tables.
