Re: selecting from partitions and constraint exclusion

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: selecting from partitions and constraint exclusion
Date: 2019-03-20 10:41:53
Message-ID: CAKJS1f-TH81=Bog7JQdxdmhcSaqKQKCpoppP6b4iuc0nitpv4A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 20 Mar 2019 at 17:37, Amit Langote
<Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> That's because get_relation_constraints() no longer (as of PG 11) includes
> the partition constraint for SELECT queries. But that's based on an
> assumption that partitions are always accessed via parent, so partition
> pruning would make loading the partition constraint unnecessary. That's
> not always true, as shown in the above example.
>
> Should we fix that? I'm attaching a patch here.

Perhaps we should. The constraint_exclusion documents [1] just mention:

> Controls the query planner's use of table constraints to optimize queries.

and I'm pretty sure you could class the partition constraint as a
table constraint.

As for the patch:

+ if ((root->parse->commandType == CMD_SELECT && !IS_OTHER_REL(rel)) ||

Shouldn't this really be checking rel->reloptkind == RELOPT_BASEREL
instead of !IS_OTHER_REL(rel) ?

For the comments:

+ * For selects, we only need those if the partition is directly mentioned
+ * in the query, that is not via parent. In case of the latter, partition
+ * pruning, which uses the parent table's partition bound descriptor,
+ * ensures that we only consider partitions whose partition constraint
+ * satisfy the query quals (or, the two don't contradict each other), so
+ * loading them is pointless.
+ *
+ * For updates and deletes, we always need those for performing partition
+ * pruning using constraint exclusion, but, only if pruning is enabled.

You mention "the latter", normally you'd only do that if there was a
former, but in this case there's not.

How about just making it:

/*
* Append partition predicates, if any.
*
* For selects, partition pruning uses the parent table's partition bound
* descriptor, so there's no need to include the partition constraint for
* this case. However, if the partition is referenced directly in the query
* then no partition pruning will occur, so we'll include it in the case.
*/
if ((root->parse->commandType != CMD_SELECT && enable_partition_pruning) ||
(root->parse->commandType == CMD_SELECT && rel->reloptkind ==
RELOPT_BASEREL))

For the tests, it seems excessive to create some new tables for this.
Won't the tables in the previous test work just fine?

[1] https://www.postgresql.org/docs/devel/runtime-config-query.html#GUC-CONSTRAINT-EXCLUSION

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Steele 2019-03-20 10:43:03 Re: Re: Planning counters in pg_stat_statements (using pgss_store)
Previous Message David Steele 2019-03-20 10:34:13 Re: Re: [RFC] [PATCH] Flexible "partition pruning" hook