Re: speeding up planning with partitions

Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> 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