Обсуждение: Should we add GUCs to allow partition pruning to be disabled?

In PG10 the planner's partition pruning could be disabled by changing
the constraint_exclusion GUC to off.  This is still the case for PG11,
but only for UPDATE and DELETE queries. There is currently no way to
disable partition pruning for SELECT.

Should we allow this?

To make this a bit more complex, we now also have run-time pruning
which can allow further partition pruning to be performed during
execution.  I imagine if we're going to add a GUC for plan-time
pruning then we should also have one for run-time pruning. These could
also perhaps share the same GUC, so it seems there are some sub
choices to make here:

1. Add a single enable_ GUC which allows both plan-time and run-time
pruning to be disabled.
2. Add two new enable_ GUCs, one for plan-time and one for run-time pruning.
3. No new GUCs / Do nothing.

Run-time pruning is a little special here too, as it's the first
feature to exist in PostgreSQL which changes the plan in the executor.
From searching through the code I see no other enable_* GUC being
referenced in the executor.  So there's also questions here as to
where we'd disable run-time pruning.  We could disable it in the
planner so that the plan does not include the details that the
executor needs to enable the pruning, or we could just disable it in
the executor and have the planner still form plans with these details.
This separates #1 and #2 into:

a) Disable run-time pruning during execution.
b) Disable run-time pruning during planning.
c) Both of the above.

The differentiation of the above is important when you consider
PREPAREd statements. Currently, no enable_ GUC will affect a
pre-PREPAREd query. We might want to keep that rule despite there
being flexibility not to, in this case.

For UPDATE/DELETE:
It would also be quite strange if someone disabled plan-time pruning
and still got partition pruning. So I suggest we require both
constraint_exclusion and the plan-time GUC not off for pruning to be
enabled for UPDATE/DELETE.  Alternatively, we just ditch
constraint_exclusion = 'partition'.

Personally, I'm for 2b and ditching constraint_exclusion =
'partition'. I don't see any sense in keeping constraint_exclusion =
'partition' if we have something else to mean the same thing.

Thoughts / Votes / Names for new GUCs?

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

On Wed, Apr 18, 2018 at 12:07:18PM +1200, David Rowley wrote:
> In PG10 the planner's partition pruning could be disabled by changing
> the constraint_exclusion GUC to off.  This is still the case for PG11,
> but only for UPDATE and DELETE queries. There is currently no way to
> disable partition pruning for SELECT.
> 
> Should we allow this?
> 
> To make this a bit more complex, we now also have run-time pruning
> which can allow further partition pruning to be performed during
> execution.  I imagine if we're going to add a GUC for plan-time
> pruning then we should also have one for run-time pruning. These could
> also perhaps share the same GUC, so it seems there are some sub
> choices to make here:
> 
> 1. Add a single enable_ GUC which allows both plan-time and run-time
> pruning to be disabled.
> 2. Add two new enable_ GUCs, one for plan-time and one for run-time pruning.
> 3. No new GUCs / Do nothing.

Maybe this is divergent from the details of the implementation; but, from a
user's perspective: why not continue to use constraint_exclusion?

I would suggest to add zero new GUCs:

0. constraint_exclusion={off,partition,on,PLANNER*,EXECUTOR*}

I tentatively assume that "constraint_exclusion=partition" would disable PG11
"pruning", and that the new default setting would be "executor".

* Caveat: there may be a better name than planner/executor..
planner_prune?  execute_filter?

Justin

On 18 April 2018 at 13:03, David G. Johnston <david.g.johnston@gmail.com> wrote:
> My initial reaction is that we need to fix the bug introduced in v10 -
> leaving constraint_exclusion working as it has historically and not affect
> the new-as-of-10 ability to prune (maybe better termed as skip...)
> partitions known during execution to contain no qualified tuples.

Can you explain which bug in PG10 you are talking about? Did you
perhaps mean PG11?

I'm not onboard with overloading the constraint_exclusion GUC any
further to mean something it shouldn't. The PG11 partition pruning
code does not use CHECK constraints to eliminate partitions, so I see
no reason why constraint_exclusion should turn it on or off.

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

On Wed, Apr 18, 2018 at 5:37 AM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
> In PG10 the planner's partition pruning could be disabled by changing
> the constraint_exclusion GUC to off.  This is still the case for PG11,
> but only for UPDATE and DELETE queries. There is currently no way to
> disable partition pruning for SELECT.
>
> Should we allow this?

I think GUC would be useful for debugging purposes for sure. Given
that we have added this feature late in v11, there might be some bugs
that will bite customers in production. It's better to provide them
some way to work-around.

>
> 1. Add a single enable_ GUC which allows both plan-time and run-time
> pruning to be disabled.

I would go for this. Both of those features have common code and it
will get cumbersome to carefully enable/disable them separately.

> 2. Add two new enable_ GUCs, one for plan-time and one for run-time pruning.

This would give more granularity but

> 3. No new GUCs / Do nothing.
>
> Run-time pruning is a little special here too, as it's the first
> feature to exist in PostgreSQL which changes the plan in the executor.
> From searching through the code I see no other enable_* GUC being
> referenced in the executor.  So there's also questions here as to
> where we'd disable run-time pruning.  We could disable it in the
> planner so that the plan does not include the details that the
> executor needs to enable the pruning, or we could just disable it in
> the executor and have the planner still form plans with these details.
> This separates #1 and #2 into:
>
> a) Disable run-time pruning during execution.
> b) Disable run-time pruning during planning.
> c) Both of the above.
>
> The differentiation of the above is important when you consider
> PREPAREd statements. Currently, no enable_ GUC will affect a
> pre-PREPAREd query. We might want to keep that rule despite there
> being flexibility not to, in this case.


If run-time pruning is disabled, why do we want to waste CPU cycles
and memory to produce plan time details? It might be useful to do so,
if there was a large chance that people prepared a statement which
could use partition pruning with run-time pruning disables but
EXECUTEd it with run-time pruning enabled. It will be less likely that
the session which prepares a plan would change the GUCs before
executing it.

>
> For UPDATE/DELETE:
> It would also be quite strange if someone disabled plan-time pruning
> and still got partition pruning. So I suggest we require both
> constraint_exclusion and the plan-time GUC not off for pruning to be
> enabled for UPDATE/DELETE.  Alternatively, we just ditch
> constraint_exclusion = 'partition'.
>
> Personally, I'm for 2b and ditching constraint_exclusion =
> 'partition'. I don't see any sense in keeping constraint_exclusion =
> 'partition' if we have something else to mean the same thing.
>

That will still be useful for inheritance based partitioning.

We might re-use constraint_exclusion = 'partition' to mean
enable_partition_pruning (ok, I suggested a name as well) = true,
although that's not my favourite.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On 18 April 2018 at 21:36, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
> On Wed, Apr 18, 2018 at 5:37 AM, David Rowley
>> a) Disable run-time pruning during execution.
>> b) Disable run-time pruning during planning.
>> c) Both of the above.
>>
>> The differentiation of the above is important when you consider
>> PREPAREd statements. Currently, no enable_ GUC will affect a
>> pre-PREPAREd query. We might want to keep that rule despite there
>> being flexibility not to, in this case.
>
>
> If run-time pruning is disabled, why do we want to waste CPU cycles
> and memory to produce plan time details? It might be useful to do so,
> if there was a large chance that people prepared a statement which
> could use partition pruning with run-time pruning disables but
> EXECUTEd it with run-time pruning enabled. It will be less likely that
> the session which prepares a plan would change the GUCs before
> executing it.

I have to admit, can't really imagine any valid cases were disabling
this feature would be useful. Generally, enable_* properties can be
used to coax the planner into producing some plan shape that it
otherwise didn't due to some costing problem.  I can only imagine it
might be useful to disable either for testing or as a workaround for
some bug that might crop up. Perhaps that's not enough reason to go
and add a GUC that'll likely need to exist forever. But it probably
does mean that we'd want c) so that the code is completely disabled as
soon as the setting is off.  If we just did it at plan time then
pre-PREPAREd queries might still prune.  That does not seem very
useful if it's being disabled due to the discovery of some bug.

The more I think about this the more undecided I am as to whether we
need to add a GUC for this at all, so I'm keen to hear more people
voice their opinion about this.  If bugs are the only true reason to
add it, then the need for the GUC should diminish every day that
nobody reports any bugs.


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

On Thu, Apr 19, 2018 at 2:54 AM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
> If we just did it at plan time then
> pre-PREPAREd queries might still prune.  That does not seem very
> useful if it's being disabled due to the discovery of some bug.
>

As you have pointed out upthread, that's a problem with every enable_*
GUC. After seeing a bug, users would usually re-prepare their
statements with pruning turned off. So, I don't see this as a reason
for introducing two GUCs.

> The more I think about this the more undecided I am as to whether we
> need to add a GUC for this at all, so I'm keen to hear more people
> voice their opinion about this.  If bugs are the only true reason to
> add it, then the need for the GUC should diminish every day that
> nobody reports any bugs.
>

Apart from bugs, I think, this GUC can be used to avoid extra planning
time/memory/CPU incurred in pruning, when users know for sure that
pruning is not going to happen e.g. the cases like no qual on
partition key or no equality qual on hash partition key etc. Do we
know how much planning time can be saved this way?

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On 2018/04/19 13:32, Ashutosh Bapat wrote:
> On Thu, Apr 19, 2018 at 2:54 AM, David Rowley
>> The more I think about this the more undecided I am as to whether we
>> need to add a GUC for this at all, so I'm keen to hear more people
>> voice their opinion about this.  If bugs are the only true reason to
>> add it, then the need for the GUC should diminish every day that
>> nobody reports any bugs.
>>
> 
> Apart from bugs, I think, this GUC can be used to avoid extra planning
> time/memory/CPU incurred in pruning, when users know for sure that
> pruning is not going to happen e.g. the cases like no qual on
> partition key or no equality qual on hash partition key etc. Do we
> know how much planning time can be saved this way?

I can imagine having a enable_partition_pruning which defaults to true, if
only to avoid the performance overhead of pruning code when a user knows
for sure that it won't help for some queries.  Although, I'm a bit dubious
why they'd write such queries if they're using partitioning in the first
place.

Also, I'd think that enable_partition_pruning set to false means pruning
doesn't occur at all, not even using constraint exclusion.  That is,
behavior equivalent of constraint_exclusion < partition (that is, off/on).

Also, if we do have such a GUC, it should apply to all command types,
including UPDATE and DELETE which don't yet invoke the new pruning code,
from the start.  So, if enable_partition_pruning is false, we won't load
the partition constraints at all, which we currently do for UPDATE and
DELETE so that constraint exclusion can be used for pruning.  OTOH, if
enable_partition_pruning is on, we perform constraint exclusion -based
pruning for UPDATE and DELETE irrespective of the setting of
constraint_exclusion GUC.  In other words, we completely dissociate
partitioned table pruning from the setting of constraint_exclusion.

Now as Justin pointed out upthread, the new GUC might cause confusion for
users who are long accustomed to using constraint_exclusion for this, but
I'm not sure anybody tries to change its setting a lot.  The new GUC
defaulting to pruning=on would be useful for occasional debugging, as we
all seem to more or less agree.

Thanks,
Amit

On Thu, Apr 19, 2018 at 5:02 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2018/04/19 13:32, Ashutosh Bapat wrote:
>> On Thu, Apr 19, 2018 at 2:54 AM, David Rowley
>>> The more I think about this the more undecided I am as to whether we
>>> need to add a GUC for this at all, so I'm keen to hear more people
>>> voice their opinion about this.  If bugs are the only true reason to
>>> add it, then the need for the GUC should diminish every day that
>>> nobody reports any bugs.
>>>
>>
>> Apart from bugs, I think, this GUC can be used to avoid extra planning
>> time/memory/CPU incurred in pruning, when users know for sure that
>> pruning is not going to happen e.g. the cases like no qual on
>> partition key or no equality qual on hash partition key etc. Do we
>> know how much planning time can be saved this way?
>
> I can imagine having a enable_partition_pruning which defaults to true, if
> only to avoid the performance overhead of pruning code when a user knows
> for sure that it won't help for some queries.  Although, I'm a bit dubious
> why they'd write such queries if they're using partitioning in the first
> place.
>
> Also, I'd think that enable_partition_pruning set to false means pruning
> doesn't occur at all, not even using constraint exclusion.  That is,
> behavior equivalent of constraint_exclusion < partition (that is, off/on).
>
> Also, if we do have such a GUC, it should apply to all command types,
> including UPDATE and DELETE which don't yet invoke the new pruning code,
> from the start.  So, if enable_partition_pruning is false, we won't load
> the partition constraints at all, which we currently do for UPDATE and
> DELETE so that constraint exclusion can be used for pruning.  OTOH, if
> enable_partition_pruning is on, we perform constraint exclusion -based
> pruning for UPDATE and DELETE irrespective of the setting of
> constraint_exclusion GUC.  In other words, we completely dissociate
> partitioned table pruning from the setting of constraint_exclusion.

Isn't word "dissociate" turns the last sentence into a sentence
contradicting everything you wrote prior to it?

I think we should keep these two things separate.
enable_partition_pruning affects the partition pruning based on the
partition bounds and that currently does not work for UPDATE/DELETE.
When it does work in those case, we might think of not loading
partition bound based constraints. constraint_exclusion affects
whether constraints can be used to exclude a relation (with partition
option affecting the child tables). Once we stop loading partition
bound based constraints, constraint exclusion would stop pruning
partitions based on the bounds. There's no point in confusing users
with by adding dependencies between these two GUCs.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On 2018/04/19 21:50, Ashutosh Bapat wrote:
> On Thu, Apr 19, 2018 at 5:02 PM, Amit Langote
>> I can imagine having a enable_partition_pruning which defaults to true, if
>> only to avoid the performance overhead of pruning code when a user knows
>> for sure that it won't help for some queries.  Although, I'm a bit dubious
>> why they'd write such queries if they're using partitioning in the first
>> place.
>>
>> Also, I'd think that enable_partition_pruning set to false means pruning
>> doesn't occur at all, not even using constraint exclusion.  That is,
>> behavior equivalent of constraint_exclusion < partition (that is, off/on).
>>
>> Also, if we do have such a GUC, it should apply to all command types,
>> including UPDATE and DELETE which don't yet invoke the new pruning code,
>> from the start.  So, if enable_partition_pruning is false, we won't load
>> the partition constraints at all, which we currently do for UPDATE and
>> DELETE so that constraint exclusion can be used for pruning.  OTOH, if
>> enable_partition_pruning is on, we perform constraint exclusion -based
>> pruning for UPDATE and DELETE irrespective of the setting of
>> constraint_exclusion GUC.  In other words, we completely dissociate
>> partitioned table pruning from the setting of constraint_exclusion.
> 
> Isn't word "dissociate" turns the last sentence into a sentence
> contradicting everything you wrote prior to it?
> 
> I think we should keep these two things separate.

Yes, that's what I meant.

To clarify: if we're going to add a new parameter *for partitioned tables*
to configure whether or not pruning occurs, even if UPDATE and DELETE now
rely on constraint exclusion for pruning, we should ignore the setting of
constraint_exclusion the configuration parameter.  For UPDATE and DELETE,
if enable_partition_pruning is on, we proceed to prune using constraint
exclusion (because that's the only method available now), irrespective of
the setting of constraint_exclusion.

So to users, enable_partition_pruning should be the only way to configure
whether or not pruning occurs.

Does that make sense?

It seems like talking about the finer implementation details is making
this discussion a bit confusing.

> enable_partition_pruning affects the partition pruning based on the
> partition bounds and that currently does not work for UPDATE/DELETE.
> When it does work in those case, we might think of not loading
> partition bound based constraints. constraint_exclusion affects
> whether constraints can be used to exclude a relation (with partition
> option affecting the child tables). Once we stop loading partition
> bound based constraints, constraint exclusion would stop pruning
> partitions based on the bounds. There's no point in confusing users
> with by adding dependencies between these two GUCs.

That's exactly what I'm trying to propose.  I don't want any new GUC to
work only for SELECT now and UPDATE/DELETE only later when we teach the
code path handling the latter to use the new pruning implementation.  In
other words, I don't want a situation where two parameters control pruning
for partitioned tables in PG 11.

BTW, should this thread be listed somewhere on the open items page?

Thanks,
Amit

On 20 April 2018 at 14:07, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> To clarify: if we're going to add a new parameter *for partitioned tables*
> to configure whether or not pruning occurs, even if UPDATE and DELETE now
> rely on constraint exclusion for pruning, we should ignore the setting of
> constraint_exclusion the configuration parameter.  For UPDATE and DELETE,
> if enable_partition_pruning is on, we proceed to prune using constraint
> exclusion (because that's the only method available now), irrespective of
> the setting of constraint_exclusion.
>
> So to users, enable_partition_pruning should be the only way to configure
> whether or not pruning occurs.
>
> Does that make sense?

So to summarise my understanding (plus filling in the blanks):

1. Add single GUC named enable_partition_pruning, default = on.
2. Check this setting in set_append_rel_size to only perform
prune_append_rel_partitions when enable_partition_pruning is true.
3. Add code in create_append_plan to only call
make_partition_pruneinfo when enable_partition_pruning is true.
4. Replace test doing (constraint_exclusion ==
CONSTRAINT_EXCLUSION_PARTITION) with (enable_partition_pruning).
5. Get rid of CONSTRAINT_EXCLUSION_PARTITION.

I don't think you mentioned 5. but if I understand you correctly then
it would leave that option doing nothing. So we should remove it.

> BTW, should this thread be listed somewhere on the open items page?

Yeah. we need to decide this before PG11 is let loose. I will add it.

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

Hi.

On 2018/04/20 11:18, David Rowley wrote:
> On 20 April 2018 at 14:07, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> To clarify: if we're going to add a new parameter *for partitioned tables*
>> to configure whether or not pruning occurs, even if UPDATE and DELETE now
>> rely on constraint exclusion for pruning, we should ignore the setting of
>> constraint_exclusion the configuration parameter.  For UPDATE and DELETE,
>> if enable_partition_pruning is on, we proceed to prune using constraint
>> exclusion (because that's the only method available now), irrespective of
>> the setting of constraint_exclusion.
>>
>> So to users, enable_partition_pruning should be the only way to configure
>> whether or not pruning occurs.
>>
>> Does that make sense?
> 
> So to summarise my understanding (plus filling in the blanks):
> 
> 1. Add single GUC named enable_partition_pruning, default = on.
> 2. Check this setting in set_append_rel_size to only perform
> prune_append_rel_partitions when enable_partition_pruning is true.
> 3. Add code in create_append_plan to only call
> make_partition_pruneinfo when enable_partition_pruning is true.
> 4. Replace test doing (constraint_exclusion ==
> CONSTRAINT_EXCLUSION_PARTITION) with (enable_partition_pruning).
> 5. Get rid of CONSTRAINT_EXCLUSION_PARTITION.
> 
> I don't think you mentioned 5. but if I understand you correctly then
> it would leave that option doing nothing. So we should remove it.

About 4 & 5:

Perhaps we should leave constraint_exclusion = partition alone because
there might be users who want to continue using the old inheritance method
to set up partitioning for whatever reason?

>> BTW, should this thread be listed somewhere on the open items page?
> 
> Yeah. we need to decide this before PG11 is let loose. I will add it.

OK, thanks.

Regards,
Amit

[1]
https://www.postgresql.org/docs/devel/static/ddl-partitioning.html#DDL-PARTITIONING-IMPLEMENTATION-INHERITANCE

On 20 April 2018 at 14:33, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2018/04/20 11:18, David Rowley wrote:
>> 4. Replace test doing (constraint_exclusion ==
>> CONSTRAINT_EXCLUSION_PARTITION) with (enable_partition_pruning).
>> 5. Get rid of CONSTRAINT_EXCLUSION_PARTITION.
>
> About 4 & 5:
>
> Perhaps we should leave constraint_exclusion = partition alone because
> there might be users who want to continue using the old inheritance method
> to set up partitioning for whatever reason?

Yeah, for some reason that keeps falling out my brain.

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

On 20 April 2018 at 14:07, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> To clarify: if we're going to add a new parameter *for partitioned tables*
> to configure whether or not pruning occurs, even if UPDATE and DELETE now
> rely on constraint exclusion for pruning, we should ignore the setting of
> constraint_exclusion the configuration parameter.  For UPDATE and DELETE,
> if enable_partition_pruning is on, we proceed to prune using constraint
> exclusion (because that's the only method available now), irrespective of
> the setting of constraint_exclusion.
>
> So to users, enable_partition_pruning should be the only way to configure
> whether or not pruning occurs.

I hope the attached implements what is being discussed here.

Please test it to ensure it behaves as you'd expect.

I was a little unsure if the new GUCs declaration should live in
costsize.c or not since it really has no effect on plan costs, but in
the end, I stuck it there anyway so that it can be with its friends.

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

On Fri, Apr 20, 2018 at 7:37 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2018/04/19 21:50, Ashutosh Bapat wrote:
>> On Thu, Apr 19, 2018 at 5:02 PM, Amit Langote
>>> I can imagine having a enable_partition_pruning which defaults to true, if
>>> only to avoid the performance overhead of pruning code when a user knows
>>> for sure that it won't help for some queries.  Although, I'm a bit dubious
>>> why they'd write such queries if they're using partitioning in the first
>>> place.
>>>
>>> Also, I'd think that enable_partition_pruning set to false means pruning
>>> doesn't occur at all, not even using constraint exclusion.  That is,
>>> behavior equivalent of constraint_exclusion < partition (that is, off/on).
>>>
>>> Also, if we do have such a GUC, it should apply to all command types,
>>> including UPDATE and DELETE which don't yet invoke the new pruning code,
>>> from the start.  So, if enable_partition_pruning is false, we won't load
>>> the partition constraints at all, which we currently do for UPDATE and
>>> DELETE so that constraint exclusion can be used for pruning.  OTOH, if
>>> enable_partition_pruning is on, we perform constraint exclusion -based
>>> pruning for UPDATE and DELETE irrespective of the setting of
>>> constraint_exclusion GUC.  In other words, we completely dissociate
>>> partitioned table pruning from the setting of constraint_exclusion.
>>
>> Isn't word "dissociate" turns the last sentence into a sentence
>> contradicting everything you wrote prior to it?
>>
>> I think we should keep these two things separate.
>
> Yes, that's what I meant.
>
> To clarify: if we're going to add a new parameter *for partitioned tables*
> to configure whether or not pruning occurs, even if UPDATE and DELETE now
> rely on constraint exclusion for pruning, we should ignore the setting of
> constraint_exclusion the configuration parameter.  For UPDATE and DELETE,
> if enable_partition_pruning is on, we proceed to prune using constraint
> exclusion (because that's the only method available now), irrespective of
> the setting of constraint_exclusion.
>
> So to users, enable_partition_pruning should be the only way to configure
> whether or not pruning occurs.
>
> Does that make sense?
>
> It seems like talking about the finer implementation details is making
> this discussion a bit confusing.
>
>> enable_partition_pruning affects the partition pruning based on the
>> partition bounds and that currently does not work for UPDATE/DELETE.
>> When it does work in those case, we might think of not loading
>> partition bound based constraints. constraint_exclusion affects
>> whether constraints can be used to exclude a relation (with partition
>> option affecting the child tables). Once we stop loading partition
>> bound based constraints, constraint exclusion would stop pruning
>> partitions based on the bounds. There's no point in confusing users
>> with by adding dependencies between these two GUCs.
>
> That's exactly what I'm trying to propose.

Not really. By pruning based on the partition bounds I didn't mean
constraint exclusion working on partition bound based constraints.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On 2018/04/20 15:00, Ashutosh Bapat wrote:
> On Fri, Apr 20, 2018 at 7:37 AM, Amit Langote wrote:
>> On 2018/04/19 21:50, Ashutosh Bapat wrote:
>>> There's no point in confusing users
>>> with by adding dependencies between these two GUCs.
>>
>> That's exactly what I'm trying to propose.
> 
> Not really. By pruning based on the partition bounds I didn't mean
> constraint exclusion working on partition bound based constraints.

Sorry, I should have said what I said after quoting only the last sentence
of what you had said.  That is, I want to the new GUC to be the only
determiner of whether the pruning occurs or not for partitioned tables.
To implement that behavior, it will have to override the setting of
constraint_exclusion (the parameter) in *some* cases, because some
commands still rely on constraint exclusion (the algorithm) as the
underlying pruning mechanism.  Now, the "override the setting of
constraint_exclusion" implementation may not be the most popular choice in
the end.

Thanks,
Amit

Hi David.

Thanks for writing the patch.

On 2018/04/20 14:47, David Rowley wrote:
> On 20 April 2018 at 14:07, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> To clarify: if we're going to add a new parameter *for partitioned tables*
>> to configure whether or not pruning occurs, even if UPDATE and DELETE now
>> rely on constraint exclusion for pruning, we should ignore the setting of
>> constraint_exclusion the configuration parameter.  For UPDATE and DELETE,
>> if enable_partition_pruning is on, we proceed to prune using constraint
>> exclusion (because that's the only method available now), irrespective of
>> the setting of constraint_exclusion.
>>
>> So to users, enable_partition_pruning should be the only way to configure
>> whether or not pruning occurs.
> 
> I hope the attached implements what is being discussed here.
> 
> Please test it to ensure it behaves as you'd expect.
> 
> I was a little unsure if the new GUCs declaration should live in
> costsize.c or not since it really has no effect on plan costs, but in
> the end, I stuck it there anyway so that it can be with its friends.

The patch looks good except one thing, which I was trying to emphasize
shouldn't be the behavior.

drop table p;
create table p (a int) partition by list (a);
create table p1 partition of p for values in (1);
create table p2 partition of p for values in (2);

set enable_partition_pruning to off;

-- ok
explain select * from p where a = 1;
                        QUERY PLAN
----------------------------------------------------------
 Append  (cost=0.00..83.88 rows=26 width=4)
   ->  Seq Scan on p1  (cost=0.00..41.88 rows=13 width=4)
         Filter: (a = 1)
   ->  Seq Scan on p2  (cost=0.00..41.88 rows=13 width=4)
         Filter: (a = 1)
(5 rows)

reset enable_partition_pruning;
-- ok
explain select * from p where a = 1;
                        QUERY PLAN
----------------------------------------------------------
 Append  (cost=0.00..41.94 rows=13 width=4)
   ->  Seq Scan on p1  (cost=0.00..41.88 rows=13 width=4)
         Filter: (a = 1)
(3 rows)

set enable_partition_pruning to off;

-- ok
explain update p set a = 2 where a = 1;
                        QUERY PLAN
-----------------------------------------------------------
 Update on p  (cost=0.00..83.75 rows=26 width=10)
   Update on p1
   Update on p2
   ->  Seq Scan on p1  (cost=0.00..41.88 rows=13 width=10)
         Filter: (a = 1)
   ->  Seq Scan on p2  (cost=0.00..41.88 rows=13 width=10)
         Filter: (a = 1)
(7 rows)

reset enable_partition_pruning;

-- ok
explain update p set a = 2 where a = 1;
                        QUERY PLAN
-----------------------------------------------------------
 Update on p  (cost=0.00..41.88 rows=13 width=10)
   Update on p1
   ->  Seq Scan on p1  (cost=0.00..41.88 rows=13 width=10)
         Filter: (a = 1)
(4 rows)


set constraint_exclusion to off;

-- not ok!
explain update p set a = 2 where a = 1;
                        QUERY PLAN
-----------------------------------------------------------
 Update on p  (cost=0.00..83.75 rows=26 width=10)
   Update on p1
   Update on p2
   ->  Seq Scan on p1  (cost=0.00..41.88 rows=13 width=10)
         Filter: (a = 1)
   ->  Seq Scan on p2  (cost=0.00..41.88 rows=13 width=10)
         Filter: (a = 1)
(7 rows)

I think we should teach relation_excluded_by_constraints() to forge ahead
based on the value of enable_partition_pruning, ignoring whatever
constraint_exclusion has been set to.  What do you think of doing that
sort of thing?

Thanks,
Amit

On 2018/04/20 17:51, Amit Langote wrote:
> On 2018/04/20 14:47, David Rowley wrote:
>> On 20 April 2018 at 14:07, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> To clarify: if we're going to add a new parameter *for partitioned tables*
>>> to configure whether or not pruning occurs, even if UPDATE and DELETE now
>>> rely on constraint exclusion for pruning, we should ignore the setting of
>>> constraint_exclusion the configuration parameter.  For UPDATE and DELETE,
>>> if enable_partition_pruning is on, we proceed to prune using constraint
>>> exclusion (because that's the only method available now), irrespective of
>>> the setting of constraint_exclusion.
>>>
>>> So to users, enable_partition_pruning should be the only way to configure
>>> whether or not pruning occurs.
>>
>> I hope the attached implements what is being discussed here.
>>
>> Please test it to ensure it behaves as you'd expect.
>>
>> I was a little unsure if the new GUCs declaration should live in
>> costsize.c or not since it really has no effect on plan costs, but in
>> the end, I stuck it there anyway so that it can be with its friends.
> 
> The patch looks good except one thing,
OK, I forgot to comment on a couple of minor issues.

+     <varlistentry id="guc-enable-partition-pruning"
xreflabel="enable_partition_pruning">
+      <term><varname>enable_partition_pruning</varname>
(<type>boolean</type>)
+       <indexterm>
+        <primary><varname>enable_partition_pruning</varname>
configuration parameter</primary>
+       </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Enables or disables the query planner's ability to eliminate a
+        partitioned table's subpartitions from query plans.

Why subpartitions?  Maybe, just "partitions" is fine.

+  This also
+        controls the planner's ability to generate query plans which
allow the
+        query executor to remove or ignoring partitions during query

Here: ignoring -> ignore

Also, maybe add the GUC to postgresql.conf.sample.

Thanks,
Amit

Amit Langote wrote:

> Sorry, I should have said what I said after quoting only the last sentence
> of what you had said.  That is, I want to the new GUC to be the only
> determiner of whether the pruning occurs or not for partitioned tables.
> To implement that behavior, it will have to override the setting of
> constraint_exclusion (the parameter) in *some* cases, because some
> commands still rely on constraint exclusion (the algorithm) as the
> underlying pruning mechanism.

I agree -- it will make more sense now, and will continue to make sense
later when we remove usage of constraint exclusion for upd/del, to make
it work as you suggest:

* if the table is partitioned, do constraint exclusion based on
  enable_partition_prune=on rather than constraint_exclusion=partition.
  This will only affect upd/del, because the select queries would be
  affected by the enable_partition_prune anyway since
  constraint_exclusion does not apply.

* If the table is using regular inheritance, continue to use the
  original behavior.

> Now, the "override the setting of constraint_exclusion" implementation
> may not be the most popular choice in the end.

I guess there are different ways to implement it.  Supposedly this is
going to disappear in pg12, so I don't think it's a big deal.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index fa92ce2e68..c51a9270e4 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -951,6 +951,15 @@ static struct config_bool ConfigureNamesBool[] =
         true,
         NULL, NULL, NULL
     },
+    {
+        {"enable_partition_pruning", PGC_USERSET, QUERY_TUNING_METHOD,
+            gettext_noop("Enables the planner's ability to remove non-required partitions from the query plan."),
+            NULL
+        },
+        &enable_partition_pruning,
+        true,
+        NULL, NULL, NULL
+    },

I would make the short description shorter, and use the long description
to elaborate.  So gettext_noop("Enable plan-time and run-time partition
pruning.")
followed by something like

gettext_noop("Allows the query planner and executor to compare partition
bounds to conditions in the query, and determine which partitions {can be
skipped | must be scanned} ...")

(Not wedded to those particular phrasings.)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Thank you both of you for looking at this.

On 21 April 2018 at 06:28, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> +               {"enable_partition_pruning", PGC_USERSET, QUERY_TUNING_METHOD,
> +                       gettext_noop("Enables the planner's ability to remove non-required partitions from the query
plan."),
> +                       NULL
> +               },
> +               &enable_partition_pruning,
> +               true,
> +               NULL, NULL, NULL
> +       },
>
> I would make the short description shorter, and use the long description
> to elaborate.  So gettext_noop("Enable plan-time and run-time partition
> pruning.")
> followed by something like
>
> gettext_noop("Allows the query planner and executor to compare partition
> bounds to conditions in the query, and determine which partitions {can be
> skipped | must be scanned} ...")

I've taken a slight variation of this, but instead of ", and" I used
"to" and went with the "must be scanned" option.

select * from pg_settings where name like 'enable%'; does show that
this is the only enable_* GUC to have a long description, but perhaps
that does not matter.

On 20 April 2018 at 20:51, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> set constraint_exclusion to off;
>
> -- not ok!

It needed a bit more effort than I put in the first time around to
make this work properly. constraint_exclusion = 'off' becomes a bit of
a special case for partitioned tables now.  To make this work I had to
get rid of hasInheritedTarget and make a new enum that tracks if we're
inheritance planning for an inheritance parent or a partitioned table.
We can't simply only set hasInheritedTarget to true when planning with
inheritance parents as constraint_exclusion = 'partition' must still
know that we're planning using the inheritance planner.

v2 patch attached.

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

Hi David.

On 2018/04/21 14:09, David Rowley wrote:
> On 20 April 2018 at 20:51, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> set constraint_exclusion to off;
>>
>> -- not ok!
> 
> It needed a bit more effort than I put in the first time around to
> make this work properly. constraint_exclusion = 'off' becomes a bit of
> a special case for partitioned tables now.  To make this work I had to
> get rid of hasInheritedTarget and make a new enum that tracks if we're
> inheritance planning for an inheritance parent or a partitioned table.
> We can't simply only set hasInheritedTarget to true when planning with
> inheritance parents as constraint_exclusion = 'partition' must still
> know that we're planning using the inheritance planner.
> 
> v2 patch attached.

Thanks for the updated patch.

Your proposed changes to inheritance_planner() look fine to me. In the
comment added by the patch in relation_excluded_by_constraints():

+ /*
+  * When constraint_exclusion is set to 'partition' we only handle
+  * OTHER_MEMBER_RELs, or BASERELs in cases where the result target is an
+  * inheritance parent or a partitioned table.
+  */

Just to clarify this a bit, would it be a good idea to be specific by
appending " (see inheritance_planner() where this is determined)" or some
such to this sentence?


BTW, while we're at it, would it also be a good idea to consider the patch
you had proposed, which I then posted an updated version of, to adjust the
documentation in ddl.sgml (in the section 5.10. Table Partitioning)
regarding the relationship between constraint exclusion and declarative
partitioning?

https://www.postgresql.org/message-id/c2838545-0e77-3c08-cd14-1c3bbf9eb62d%40lab.ntt.co.jp

Thanks,
Amit

Hi,

I just pushed David patch, with some pretty minor changes.  I hope not
to have broken anything.

Amit Langote wrote:

> Your proposed changes to inheritance_planner() look fine to me. In the
> comment added by the patch in relation_excluded_by_constraints():
> 
> + /*
> +  * When constraint_exclusion is set to 'partition' we only handle
> +  * OTHER_MEMBER_RELs, or BASERELs in cases where the result target is an
> +  * inheritance parent or a partitioned table.
> +  */
> 
> Just to clarify this a bit, would it be a good idea to be specific by
> appending " (see inheritance_planner() where this is determined)" or some
> such to this sentence?

I didn't think that was really required.

> BTW, while we're at it, would it also be a good idea to consider the patch
> you had proposed, which I then posted an updated version of, to adjust the
> documentation in ddl.sgml (in the section 5.10. Table Partitioning)
> regarding the relationship between constraint exclusion and declarative
> partitioning?

I looked at this one.  That patch has two hunks.  I applied a change
where the first hunk is, to replace constraint_exclusion with the new
GUC -- seemed easy enough.  However, the second hunk is on "5.10.4.
Partitioning and Constraint Exclusion" which needs major editing.  Not
really sure how best to handle that one.  For starters, I think it need
to stop mentioning the GUC name in the title; maybe rename it to
"Partition Pruning" or some such, and then in the text explain that
sometimes the enable_partition_pruning param is used in one case and
constraint_exclusion in the other, and approximately what effects they
have.  I don't think it's worth going into too much detail on exactly
how they differ, but then I'm not 100% sure of that either.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2018/04/24 6:10, Alvaro Herrera wrote:
>> BTW, while we're at it, would it also be a good idea to consider the patch
>> you had proposed, which I then posted an updated version of, to adjust the
>> documentation in ddl.sgml (in the section 5.10. Table Partitioning)
>> regarding the relationship between constraint exclusion and declarative
>> partitioning?
> 
> I looked at this one.  That patch has two hunks.  I applied a change
> where the first hunk is, to replace constraint_exclusion with the new
> GUC -- seemed easy enough.

Looks good.

> However, the second hunk is on "5.10.4.
> Partitioning and Constraint Exclusion" which needs major editing.

Reading 5.10.4 again, I tend to agree with this.

> Not really sure how best to handle that one.  For starters, I think it need
> to stop mentioning the GUC name in the title;

Hmm, "Constraint Exclusion" that's used in the title is a concept, not a
GUC, although pretty close.

> maybe rename it to
> "Partition Pruning" or some such, and then in the text explain that
> sometimes the enable_partition_pruning param is used in one case and
> constraint_exclusion in the other, and approximately what effects they
> have.  I don't think it's worth going into too much detail on exactly
> how they differ, but then I'm not 100% sure of that either.

Just a thought -- How about making 5.10.4 cover partitioning based
optimizations in general?  I see that a number of partitioning-based
optimizations have been developed in this release cycle, but I only see
various enable_partition* GUCs listed in config.sgml and not much else.
Although the config.sgml coverage of the new capabilities seems pretty
good, some may find their being mentioned in 5.10 Table Partitioning
helpful.  Or if we don't want to hijack 5.10.4, maybe create a 5.10.5.

Thanks,
Amit

The patch looks good to me, David.




--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html

Amit Langote wrote:
> On 2018/04/24 6:10, Alvaro Herrera wrote:

> > Not really sure how best to handle that one.  For starters, I think it need
> > to stop mentioning the GUC name in the title;
> 
> Hmm, "Constraint Exclusion" that's used in the title is a concept, not a
> GUC, although pretty close.

Yeah, I meant that if we want that section to cover the general concept
of partition pruning, with either technique, better not use the words
"constraint exclusion" in the title.

> > maybe rename it to
> > "Partition Pruning" or some such, and then in the text explain that
> > sometimes the enable_partition_pruning param is used in one case and
> > constraint_exclusion in the other, and approximately what effects they
> > have.  I don't think it's worth going into too much detail on exactly
> > how they differ, but then I'm not 100% sure of that either.
> 
> Just a thought -- How about making 5.10.4 cover partitioning based
> optimizations in general?  I see that a number of partitioning-based
> optimizations have been developed in this release cycle, but I only see
> various enable_partition* GUCs listed in config.sgml and not much else.

I think we should not rely on the config.sgml blurbs exclusively; some
narrative is always welcome -- except that for planner enable_* settings
I'm not sure we really need all that much text after all.  Constraint
exclusion was pretty easy to get wrong, hence the need for a separate
section, and I suppose the new partition pruning may be prey to the same
problems, so it seems worth to document them specially.  But not sure
about the others, if they are mostly debugging tools.

> Although the config.sgml coverage of the new capabilities seems pretty
> good, some may find their being mentioned in 5.10 Table Partitioning
> helpful.  Or if we don't want to hijack 5.10.4, maybe create a 5.10.5.

Can you (or someone) describe what would that section contain?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Jim Finnerty wrote:
> The patch looks good to me, David.
> 

Thanks for checking!  It's already pushed.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 24 April 2018 at 09:10, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> I just pushed David patch, with some pretty minor changes.  I hope not
> to have broken anything.

Thanks for pushing and thanks Amit for reviewing.

The only thing that stands out in the actual commit is:

+        executor to remove (ignore) partitions during query execution.  The

I had originally written:

+        executor to remove or ignore partitions during query execution.  The

The reason I was using "remove or ignore" was that partitions pruned
during init plan are effectively "removed" from the plan, whereas
partitions pruned during the running of the planner are just
"ignored".

It's minor details but I thought I'd better point it out.

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

On 25 April 2018 at 09:59, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Amit Langote wrote:
>> Although the config.sgml coverage of the new capabilities seems pretty
>> good, some may find their being mentioned in 5.10 Table Partitioning
>> helpful.  Or if we don't want to hijack 5.10.4, maybe create a 5.10.5.
>
> Can you (or someone) describe what would that section contain?

I've drafted and attached a patch of how I think this should look.
Likely it will need some tweaking, but it is probably a good starting
point for discussion.

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

On 2018/04/26 16:29, David Rowley wrote:
> On 25 April 2018 at 09:59, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> Amit Langote wrote:
>>> Although the config.sgml coverage of the new capabilities seems pretty
>>> good, some may find their being mentioned in 5.10 Table Partitioning
>>> helpful.  Or if we don't want to hijack 5.10.4, maybe create a 5.10.5.
>>
>> Can you (or someone) describe what would that section contain?
> 
> I've drafted and attached a patch of how I think this should look.
> Likely it will need some tweaking, but it is probably a good starting
> point for discussion.

Thanks David for drafting this.  I see that you have not included the
description of other partitioning-based optimizations such partitionwise
plans that the planner can now consider.  I guess that's fine though, as
Alvaro also seemed a bit doubtful about the value of providing the
description of those optimizations in this part of the documentation (that
is, in 5.10 Table Partitioning).

About the patch in general, it seems like the newly added documentation
talks about "Partition Pruning" as something that *replaces* constraint
exclusion.  But, I think "Partition Pruning" is not the thing that
replaces constraint exclusion.  We used to do partition pruning even
before and used constraint exclusion as the algorithm.  What's new is the
algorithm that we now use to perform partition pruning for declaratively
partitioned tables.  Also, the characteristics of the new algorithm are
such that it can now be used in more situations, thus making it more
useful than the earlier method of partition pruning, so that new features
like runtime pruning could be realized.  I like that the patch adds
various details about the new pruning features, but think that the wording
and the flow could be improved a bit.

What do you think?

Thanks,
Amit

On Tue, Apr 24, 2018 at 5:59 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Constraint
> exclusion was pretty easy to get wrong, hence the need for a separate
> section, and I suppose the new partition pruning may be prey to the same
> problems, so it seems worth to document them specially.  But not sure
> about the others, if they are mostly debugging tools.

Weighing in here late, but I have a hard time understanding why we
want a GUC to control partition pruning at all. With constraint
exclusion, the issue is whether you want to spend planner cycles to
try to deduce things using CHECK constraints when, quite possibly,
your CHECK constraints are unrelated to table inheritance and thus
won't help.  But seems extremely unlikely that the same thing would
happen with partition pruning.  Unlike your CHECK constraints, your
partition bounds are, by definition, potentially useful for pruning.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2 May 2018 at 09:14, David Rowley <david.rowley@2ndquadrant.com> wrote:
> It might be worth running a series of benchmarks to test where the worst case performance hit is with partition
pruning.

I just did this:

Setup:

create table parttable (a int, b int) partition by list (a);
create table parttable1 partition of parttable for values in(1);

Benchmark:
$ echo "select * from parttable where b
in(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20);" >
parttable.sql
$ pgbench -T 30 -f parttable.sql -n postgres
transaction type: parttable.sql
scaling factor: 1
query mode: simple
number of clients: 1
number of threads: 1
duration: 30 s
number of transactions actually processed: 230908
latency average = 0.130 ms
tps = 7696.884795 (including connections establishing)
tps = 7697.304782 (excluding connections establishing)

$ psql -c "alter system set enable_partition_pruning = off" postgres
ALTER SYSTEM

$ pgbench -T 30 -f parttable.sql -n postgres
transaction type: parttable.sql
scaling factor: 1
query mode: simple
number of clients: 1
number of threads: 1
duration: 30 s
number of transactions actually processed: 233545
latency average = 0.128 ms
tps = 7784.800130 (including connections establishing)
tps = 7785.225490 (excluding connections establishing)

So about 1.1% performance improvement. That's not very much.

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

Hi Amit,

Thanks for looking at the patch.

On 1 May 2018 at 21:44, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> About the patch in general, it seems like the newly added documentation
> talks about "Partition Pruning" as something that *replaces* constraint
> exclusion.  But, I think "Partition Pruning" is not the thing that
> replaces constraint exclusion.

Not sure where you see the mention partition pruning replacing
constraint exclusion.

> We used to do partition pruning even
> before and used constraint exclusion as the algorithm.

That depends on if you think of partition pruning as the new feature
or the act of removing unneeded partitions. We seem to have settled on
partition pruning being the new feature given that we named the GUC
this way. So I don't quite understand what you mean here.

>  What's new is the
> algorithm that we now use to perform partition pruning for declaratively
> partitioned tables.  Also, the characteristics of the new algorithm are
> such that it can now be used in more situations, thus making it more
> useful than the earlier method of partition pruning, so that new features
> like runtime pruning could be realized.  I like that the patch adds
> various details about the new pruning features, but think that the wording
> and the flow could be improved a bit.
>
> What do you think?

I re-read the patch and it still looks fine to me. I'm sure it could
be made better, but I just don't currently see how. I think it would
be better if you commented on the specifics of what you think could be
improved rather than a general comment that it could be improved.

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

Hi David.

On 2018/05/02 8:18, David Rowley wrote:
> On 1 May 2018 at 21:44, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> About the patch in general, it seems like the newly added documentation
>> talks about "Partition Pruning" as something that *replaces* constraint
>> exclusion.  But, I think "Partition Pruning" is not the thing that
>> replaces constraint exclusion.
> 
> Not sure where you see the mention partition pruning replacing
> constraint exclusion.
> 
>> We used to do partition pruning even
>> before and used constraint exclusion as the algorithm.
> 
> That depends on if you think of partition pruning as the new feature
> or the act of removing unneeded partitions. We seem to have settled on
> partition pruning being the new feature given that we named the GUC
> this way. So I don't quite understand what you mean here.
> 
>>  What's new is the
>> algorithm that we now use to perform partition pruning for declaratively
>> partitioned tables.  Also, the characteristics of the new algorithm are
>> such that it can now be used in more situations, thus making it more
>> useful than the earlier method of partition pruning, so that new features
>> like runtime pruning could be realized.  I like that the patch adds
>> various details about the new pruning features, but think that the wording
>> and the flow could be improved a bit.
>>
>> What do you think?
> 
> I re-read the patch and it still looks fine to me. I'm sure it could
> be made better, but I just don't currently see how. I think it would
> be better if you commented on the specifics of what you think could be
> improved rather than a general comment that it could be improved.

Sorry, I may have been a bit vague.  I've read the patch one more time by
considering the phrase "partition pruning" as the name of the new feature
and that constraint exclusion is an optimization technique which doubled
as partition pruning until now.  The new feature achieves results faster
and can be used in more cases than constraint exclusion.  With that
reading, I don't see much to complain about with your patch at a high level.

Except some nitpicking:

+   <para>
+    Partition Pruning is also more powerful than constraint exclusion as
+    partition pruning is not something that is performed only during the
+    planning of a given query.

Maybe, don't repeat "partition pruning" again in the same sentence.  How
about:

.. more powerful than constraint exclusion as *it* is not something..

Or may suggest to rewrite it as:

Partition pruning is also more powerful than constraint exclusion as it
can be performed not only during the planning of a given query, but also
during its execution.

If you accept the above rewrite, the next sentences in the paragraph:

+    In certain cases, partition pruning may also
+    be performed during execution of the query as well.  This allows pruning
+    to be performed using values which are unknown during query planning, for
+    example, using parameters defined in a <command>PREPARE</command>
+    statement, using a value obtained from a subquery or using parameters
from
+    a parameterized nested loop join.

could be adjusted a bit to read as:

For example, this allows pruning to be performed using values which are
unknown during query planning but will be known during execution, such as
using parameters defined in a <command>PREPARE</command> statement (if a
generic plan is chosen), or using a value obtained from a subquery, or
using values from an outer row of a parameterized nested loop join.

+   <para>
+    The partition pruning which is performed during execution is done so at
+    either one or both of the following times:

done so at -> done at

+       If partition pruning can be
+       performed here then there is the added benefit of not having to
+       initialize partitions which are pruned.  Partitions which are pruned
+       during this stage will not show up in the query's
+       <command>EXPLAIN</command> or <command>EXPLAIN ANALYZE</command>.  It
+       is possible to determine the number of partitions which were removed
+       using this method by observing the <quote>Subplans Removed</quote>
+       property in the <command>EXPLAIN</command> output.

While it might be OK to keep the last two sentences, not sure about the
1st, which seems like it's spelling out an implementation detail -- that
there is an initialization step for partitions.  It's a nice performance
enhancement, sure, but might be irrelevant to the users reading this
documentation.

+       nested loop joins.  Since the value of these parameters may change
many
+       times during the execution of the query, partition pruning is
performed
+       whenever one of the execution parameters which is being compared to a
+       partition column or expression changes.

How about writing the last part as: whenever one of the execution
parameters relevant to pruning changes

+   <note>
+    <para>
+     Currently, partition pruning of partitions during the planning of an
+     <command>UPDATE</command> or <command>DELETE</command> command are
+     internally implemented using the constraint exclusion method.  Only
+     <command>SELECT</command> uses the faster partition pruning method.
Also
+     partition pruning performed during execution is only done so for the
+     Append node type.  Both of these limitations are likely to be removed
+     in a future release of <productname>PostgreSQL</productname>.
+    </para>
+   </note>

Do we need to write this given that we decided to decouple even the
UPDATE/DELETE pruning from the constraint_exclusion configuration?  Also,
noting that only Append nodes can use execution-time pruning seems
unnecessary.  I don't see plan node names mentioned like this elsewhere in
the documentation.  But more to the point, it seems like spilling out
finer implementation details (and/or limitations thereof) in the
user-facing documentation.

Thanks again.

Regards,
Amit

Robert Haas wrote:
> On Tue, Apr 24, 2018 at 5:59 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > Constraint
> > exclusion was pretty easy to get wrong, hence the need for a separate
> > section, and I suppose the new partition pruning may be prey to the same
> > problems, so it seems worth to document them specially.  But not sure
> > about the others, if they are mostly debugging tools.
> 
> Weighing in here late, but I have a hard time understanding why we
> want a GUC to control partition pruning at all. With constraint
> exclusion, the issue is whether you want to spend planner cycles to
> try to deduce things using CHECK constraints when, quite possibly,
> your CHECK constraints are unrelated to table inheritance and thus
> won't help.  But seems extremely unlikely that the same thing would
> happen with partition pruning.  Unlike your CHECK constraints, your
> partition bounds are, by definition, potentially useful for pruning.

I admit I am more concerned about the possibility of bugs than I am
about providing a performance-related tool.  If partition prune can do
its thing with only a 1.1% of overhead, that's a great result.  While
I'm sure that some real-world partitioning scenarios exist that have a
higher overhead than that, that's not what I am worried about the most.

In a couple of releases, once we know for sure that all this new code is
absolutely stable and that there are no bugs (keeping in mind that PG12
will boast additional pruning for MergeAppend as well as for UPDATE/
DELETE queries,) we can remove the GUC -- hoping that no user will bark
at us about they having to keep it disabled by default.  

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 1 May 2018 at 21:44, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> About the patch in general, it seems like the newly added documentation
> talks about "Partition Pruning" as something that *replaces* constraint
> exclusion.  But, I think "Partition Pruning" is not the thing that
> replaces constraint exclusion.

Just thinking about this a bit more. I've become a bit concerned that
we've completely misnamed this feature. It's true that at the moment
we build RelOptInfos for all partitions then eliminate what we can,
but the new algorithm that we've been calling "partition pruning" is
really not pruning anything at all, it's selecting the smallest set of
matching partitions. It's only the current usage of the algorithm
that's using it that way, and I kinda hope to change that for PG12.

Isn't the whole thing better to be named "partition selection"?

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

On 3 May 2018 at 11:38, David G. Johnston <david.g.johnston@gmail.com> wrote:
> Maybe "Partition Filtering" (I'm disliking selection, I'm thinking we must
> always select partitions)

I don't see why "Filtering" is any different from pruning, they both
imply removing something that was once there. What I'm saying is, that
it's backward to think of what we have now as pruning, so I don't
think renaming it to "partition filtering" addresses my concern.

FWIW, I'm not set on changing this. I just want to discuss this now so
that the chances of having regrets about this later are reduced.


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

On Wed, May 2, 2018 at 9:28 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> I admit I am more concerned about the possibility of bugs than I am
> about providing a performance-related tool.

I agree that if partition pruning has bugs, somebody might want to
turn it off.  On the other hand, when they do, there's a good chance
that they will lose so much performance that they'll still be pretty
sad.  Somebody certainly could have a workload where the pruning
helps, but by a small enough amount that shutting it off is
acceptable.  But I suspect that's a somewhat narrow target.

I'm not going to go to war over this, though.  I'm just telling you
what I think.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas wrote:
> On Wed, May 2, 2018 at 9:28 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > I admit I am more concerned about the possibility of bugs than I am
> > about providing a performance-related tool.
> 
> I agree that if partition pruning has bugs, somebody might want to
> turn it off.  On the other hand, when they do, there's a good chance
> that they will lose so much performance that they'll still be pretty
> sad.  Somebody certainly could have a workload where the pruning
> helps, but by a small enough amount that shutting it off is
> acceptable.  But I suspect that's a somewhat narrow target.
> 
> I'm not going to go to war over this, though.  I'm just telling you
> what I think.

Well, we didn't have a GUC initially, evidently because none of us
thought that this would be a huge problem.  So maybe you're both right
and it's overkill to have it.  I'm not set on having it, either.  Does
anybody else have an opinion?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Robert Haas wrote:
> On Wed, May 2, 2018 at 9:28 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > I admit I am more concerned about the possibility of bugs than I am
> > about providing a performance-related tool.
>
> I agree that if partition pruning has bugs, somebody might want to
> turn it off.  On the other hand, when they do, there's a good chance
> that they will lose so much performance that they'll still be pretty
> sad.  Somebody certainly could have a workload where the pruning
> helps, but by a small enough amount that shutting it off is
> acceptable.  But I suspect that's a somewhat narrow target.
>
> I'm not going to go to war over this, though.  I'm just telling you
> what I think.

Well, we didn't have a GUC initially, evidently because none of us
thought that this would be a huge problem.  So maybe you're both right
and it's overkill to have it.  I'm not set on having it, either.  Does
anybody else have an opinion?

On Thu, Apr 26, 2018 at 07:29:37PM +1200, David Rowley wrote:
> On 25 April 2018 at 09:59, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > Amit Langote wrote:
> >> Although the config.sgml coverage of the new capabilities seems pretty
> >> good, some may find their being mentioned in 5.10 Table Partitioning
> >> helpful.  Or if we don't want to hijack 5.10.4, maybe create a 5.10.5.
> >
> > Can you (or someone) describe what would that section contain?
> 
> I've drafted and attached a patch of how I think this should look.
> Likely it will need some tweaking, but it is probably a good starting
> point for discussion.

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 34da0d8d57..89735b4804 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml

+   <para>
+    Unlike constraint exclusion, partition pruning can be performed much more
+    quickly as it does not have to scan each individual partition's metadata
quickly COMMA

But actually I suggest:
Partition pruning is much more efficient than constraint exclusion, since
pruning avoids scanning each partition's metadata...


+    Partition Pruning is also more powerful than constraint exclusion as
+    partition pruning is not something that is performed only during the
remove "something that is" ?
Or just merge into the next sentence.
Note: Amit and David commented on this previously.

+    planning of a given query.  In certain cases, partition pruning may also
+    be performed during execution of the query as well.  This allows pruning
"also" is redundant with "as well"

+    to be performed using values which are unknown during query planning, for
could say "are not yet known"

+    The partition pruning which is performed during execution is done so at
+    either one or both of the following times:
remove "either" ?

+       During initialization of the query plan.  Partition pruning can be
+       initialization phase of execution.  If partition pruning can be
+       performed here then there is the added benefit of not having to
here COMMA

+       initialize partitions which are pruned.  Partitions which are pruned
+       during this stage will not show up in the query's

+       During actual execution of the query plan.  Partition pruning may also
Remove "actual" ?

+       be performed here to remove partitions using values which are only known
+       during actual query execution.  This includes values from subqueries and

+       values from execution time parameters such as ones from parameterized
execution-time?
s/ones/those/

+       partition column or expression changes.  In order to determine if
+       partitions were pruned at this stage requires careful inspection of the
+       <literal>nloops</literal> property in the
+       <command>EXPLAIN ANALYZE</command> output.
s/In order to determine/Determining/

+    <para>
+     Currently, partition pruning of partitions during the planning of an
s/partition //1 (just "pruning of partitions")

+     <command>UPDATE</command> or <command>DELETE</command> command are
s/are/is/

+     internally implemented using the constraint exclusion method.  Only
remove "internally"?

+     <command>SELECT</command> uses the faster partition pruning method.  Also
Also COMMA

+     partition pruning performed during execution is only done so for the
Remove "so".

Justin

Thanks for looking at this. I've taken most of your suggestions, some
I had changed around as a result of Amit's review.

On 7 May 2018 at 15:34, Justin Pryzby <pryzby@telsasoft.com> wrote:
> +       During actual execution of the query plan.  Partition pruning may also
> Remove "actual" ?

I left this out one. I imagined it would be common to think of
executor startup/execution/execution shutdown as "query execution",
but I really only mean during the middle of those three things.  If
you can think of a better way to make that more clear, then it might
be worth considering. For now, I think removing "actual" won't help.

A patch will follow shortly, in response to Amit's review.

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

Many thanks for reviewing this.

On 2 May 2018 at 20:07, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> +   <para>
> +    Partition Pruning is also more powerful than constraint exclusion as
> +    partition pruning is not something that is performed only during the
> +    planning of a given query.
>
> Maybe, don't repeat "partition pruning" again in the same sentence.  How
> about:
>
> .. more powerful than constraint exclusion as *it* is not something..

changed.

> Or may suggest to rewrite it as:
>
> Partition pruning is also more powerful than constraint exclusion as it
> can be performed not only during the planning of a given query, but also
> during its execution.
>
> If you accept the above rewrite, the next sentences in the paragraph:
>
> +    In certain cases, partition pruning may also
> +    be performed during execution of the query as well.  This allows pruning
> +    to be performed using values which are unknown during query planning, for
> +    example, using parameters defined in a <command>PREPARE</command>
> +    statement, using a value obtained from a subquery or using parameters
> from
> +    a parameterized nested loop join.
>
> could be adjusted a bit to read as:
>
> For example, this allows pruning to be performed using values which are
> unknown during query planning but will be known during execution, such as
> using parameters defined in a <command>PREPARE</command> statement (if a
> generic plan is chosen), or using a value obtained from a subquery, or
> using values from an outer row of a parameterized nested loop join.

I've changed this a bit but I didn't mention generic plans. What you
say is true, but I didn't think we needed to be so specific.

> +   <para>
> +    The partition pruning which is performed during execution is done so at
> +    either one or both of the following times:
>
> done so at -> done at

Changed

> +       If partition pruning can be
> +       performed here then there is the added benefit of not having to
> +       initialize partitions which are pruned.  Partitions which are pruned
> +       during this stage will not show up in the query's
> +       <command>EXPLAIN</command> or <command>EXPLAIN ANALYZE</command>.  It
> +       is possible to determine the number of partitions which were removed
> +       using this method by observing the <quote>Subplans Removed</quote>
> +       property in the <command>EXPLAIN</command> output.
>
> While it might be OK to keep the last two sentences, not sure about the
> 1st, which seems like it's spelling out an implementation detail -- that
> there is an initialization step for partitions.  It's a nice performance
> enhancement, sure, but might be irrelevant to the users reading this
> documentation.

I've reworded this. I think it's important to inform the reader that
this is performed during initialization of the plan as without that
they might ask why there are two phases of pruning and not just one.
Not having to initialize the subnode for pruned partitions is the sole
advantage of doing this pruning phase, so I would rather be specific
about when it occurs.

> +       nested loop joins.  Since the value of these parameters may change
> many
> +       times during the execution of the query, partition pruning is
> performed
> +       whenever one of the execution parameters which is being compared to a
> +       partition column or expression changes.
>
> How about writing the last part as: whenever one of the execution
> parameters relevant to pruning changes

I've reworded this.

> +   <note>
> +    <para>
> +     Currently, partition pruning of partitions during the planning of an
> +     <command>UPDATE</command> or <command>DELETE</command> command are
> +     internally implemented using the constraint exclusion method.  Only
> +     <command>SELECT</command> uses the faster partition pruning method.
> Also
> +     partition pruning performed during execution is only done so for the
> +     Append node type.  Both of these limitations are likely to be removed
> +     in a future release of <productname>PostgreSQL</productname>.
> +    </para>
> +   </note>
>
> Do we need to write this given that we decided to decouple even the
> UPDATE/DELETE pruning from the constraint_exclusion configuration?

I think it's important to inform people of the limitations. I know
there's a lot of opinions floating around about the usability of
partitioning in PostgreSQL with a large number of partitions. I
included this here so interested parties know that their problems are
not all solved by partition pruning. Perhaps those people can watch
for the removal of this notice.

>  Also,
> noting that only Append nodes can use execution-time pruning seems
> unnecessary.  I don't see plan node names mentioned like this elsewhere in
> the documentation.  But more to the point, it seems like spilling out
> finer implementation details (and/or limitations thereof) in the
> user-facing documentation.

I thought about this while writing the patch, and it forced me to grep
for instances of "Append" in the docs.  There were some, so I didn't
think I was breaking any rules.  I also have no idea how else we might
explain that it works for Append and not MergeAppend. It's likely
going to be easier to answer possible to future bug reports which
complain run-time pruning is broken with MergeAppend with "It's not a
bug, it's behaving exactly as described in the documents.  <link to
docs>".

I have now changed Append for <literal>Append</literal> in the patch
which is aligned to what perform.sgml is doing.

v2 patch is attached.

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

On Mon, May 07, 2018 at 06:00:59PM +1200, David Rowley wrote:
> Many thanks for reviewing this.

2nd round - from the minimalist department:

+    partitions which cannot possibly contain any matching records.
maybe: partitions which cannot match any records.

+   <para>
+    Partition pruning done during execution can be performed at any of the
+    following times:

remove "done"?

+       number of partitions which were removed during this phase of pruning by
remove "of prunning"

Justin

Thanks for reviewing again.

On 9 May 2018 at 01:32, Justin Pryzby <pryzby@telsasoft.com> wrote:
> On Mon, May 07, 2018 at 06:00:59PM +1200, David Rowley wrote:
>> Many thanks for reviewing this.
>
> 2nd round - from the minimalist department:
>
> +    partitions which cannot possibly contain any matching records.
> maybe: partitions which cannot match any records.

I don't think that's an improvement. I don't think there's such a
thing as "partitions which match records". A partition can contain a
record, it never matches one.

> +   <para>
> +    Partition pruning done during execution can be performed at any of the
> +    following times:
>
> remove "done"?

Removed.

> +       number of partitions which were removed during this phase of pruning by
> remove "of prunning"

Removed.

v3 attached.

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

Hi David.

Thanks for addressing my comments.

On 2018/05/07 15:00, David Rowley wrote:
> v2 patch is attached.

Looks good to me.

Thanks,
Amit

On 2018/05/09 13:14, Amit Langote wrote:
> Hi David.
> 
> Thanks for addressing my comments.
> 
> On 2018/05/07 15:00, David Rowley wrote:
>> v2 patch is attached.
> 
> Looks good to me.

Sorry, I should've seen noticed v3 before sending my email.

v3 looks good too, but when going through it, I noticed one bit in 5.10.4.
Partitioning and Constraint Exclusion:

         A good rule of thumb is that partitioning constraints should
      contain only comparisons of the partitioning column(s) to constants
      using B-tree-indexable operators, which applies even to partitioned
      tables, because only B-tree-indexable column(s) are allowed in the
      partition key.

I think the part after ", which applies even to partitioned tables,.."
should be removed.

Attached find the updated patch.

Thanks,
Amit

David Rowley wrote:
> Thanks for reviewing again.

Hi,

I'm thinking something a bit more radical.  First, since partition
pruning is the future and constraint exclusion is soon to be a thing of
the past, we should describe pruning first, and then describe exclusion
in terms of pruning.  Second, I'd put constraint exclusion as a <sect3>
inside the <sect2> that describes pruning (but keep the XML "id" the
same, so that old links continue to work.)

I took a stab at this, but ran out of time before trimming the text for
constraint exclusion.  What do you think of this rough sketch?  I'm
thinking 5.10.4 is close to its final form (wording suggestions of
course welcome), but 5.10.4.1 still needs to be trimmed heavily, to
avoid repeating what was already explained in 5.10.4 (we need only
explain how exclusion differs from pruning.)

I'm a bit undecided on where to leave the <note>.

(Note:
   make -C doc/src/sgml html XSLTPROCFLAGS='--stringparam rootid ddl'
builds only the 'ddl' chapter, which is nice when proofreading.)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 10 May 2018 at 14:01, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> I'm thinking something a bit more radical.  First, since partition
> pruning is the future and constraint exclusion is soon to be a thing of
> the past, we should describe pruning first, and then describe exclusion
> in terms of pruning.

But... that's not true.  The chapter describes inheritance partitioned
tables too, and we're not getting rid of constraint exclusion because
it's needed for those. However, that might not mean your patch has to
be changed. I'd better have a look...

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

David Rowley wrote:
> On 10 May 2018 at 14:01, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > I'm thinking something a bit more radical.  First, since partition
> > pruning is the future and constraint exclusion is soon to be a thing of
> > the past, we should describe pruning first, and then describe exclusion
> > in terms of pruning.
> 
> But... that's not true.  The chapter describes inheritance partitioned
> tables too, and we're not getting rid of constraint exclusion because
> it's needed for those.

Oh, I'm sure it is, but nobody is going to set up new inheritance
partitioned tables anymore, except people who pg_upgrade from older
releases.  (And while I haven't tried, I'm sure it's possible to migrate
from old-style to new-style partitioned tables without incurring full
table rewrites, with little downside and lots to gain.)

Now, maybe you argue that we could have a structure like this instead:

5.10.1. Overview
5.10.2. Declarative Partitioning
5.10.3. Partition Pruning
5.10.4. Implementation Using Inheritance
5.10.5. Constraint Exclusion

I wouldn't oppose that.

> However, that might not mean your patch has to be changed. I'd better
> have a look...

Thanks :-)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Wed, May 9, 2018 at 10:10 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
> On 10 May 2018 at 14:01, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>> I'm thinking something a bit more radical.  First, since partition
>> pruning is the future and constraint exclusion is soon to be a thing of
>> the past, we should describe pruning first, and then describe exclusion
>> in terms of pruning.
>
> But... that's not true.  The chapter describes inheritance partitioned
> tables too, and we're not getting rid of constraint exclusion because
> it's needed for those. However, that might not mean your patch has to
> be changed. I'd better have a look...

I agree that constraint exclusion isn't going to die any time soon,
but I think Alvaro is right to say that we should explain the new
partition pruning technique first, and then later explain, hey, we
have this constraint exclusion thing, too.  It takes more work to
reorganize the documentation along those lines, but nobody wants to
read about the techniques in the order we implemented them.  They want
to read about the important stuff first, and in this case, that's the
new form of partition pruning.

In defense of constraint exclusion, let me note that constraint
exclusion is not restricted to inheritance cases.  It could eliminate
the need to scan a completely unpartitioned table if the WHERE clause
can be refuted by CHECK constraints.  It could eliminate the need to
scan some partitions of a partitioned table based on whatever
additional CHECK constraints exist beyond the partitioning
constraints.  These are less likely scenarios, perhaps, but not out of
the question.  For example, imagine a partitioned order table that is
range-partitioned by order ID.  You could add CHECK constraints based
on the order_date that appears in each partition, and then constraint
exclusion could eliminate partitions based on quals related to
order_date.  The order date correlates with the order ID, but
partition pruning doesn't know that, so it can can only help with
quals based on order ID.  Constraint exclusion doesn't have that
restriction.  That's potentially useful, I think, although BRIN
indexes on each partition are another way to tackle this sort of
problem.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas wrote:

> In defense of constraint exclusion, let me note that constraint
> exclusion is not restricted to inheritance cases.  It could eliminate
> the need to scan a completely unpartitioned table if the WHERE clause
> can be refuted by CHECK constraints.  It could eliminate the need to
> scan some partitions of a partitioned table based on whatever
> additional CHECK constraints exist beyond the partitioning
> constraints.

This is a great point that hadn't occurred to me.  It means that we
should keep constraint exclusion on its own <sect2> rather than relegate
it to <sect3>, as my proposed patch does.  I think it's a good idea to
add this point there too.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

David Rowley wrote:
> On 1 May 2018 at 21:44, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> > About the patch in general, it seems like the newly added documentation
> > talks about "Partition Pruning" as something that *replaces* constraint
> > exclusion.  But, I think "Partition Pruning" is not the thing that
> > replaces constraint exclusion.
> 
> Just thinking about this a bit more. I've become a bit concerned that
> we've completely misnamed this feature. It's true that at the moment
> we build RelOptInfos for all partitions then eliminate what we can,
> but the new algorithm that we've been calling "partition pruning" is
> really not pruning anything at all, it's selecting the smallest set of
> matching partitions. It's only the current usage of the algorithm
> that's using it that way, and I kinda hope to change that for PG12.
> 
> Isn't the whole thing better to be named "partition selection"?

I think that approach makes it more difficult to explain, not less so.

There are two logically opposite ways to explain this feature: a) by
default, all partitions must be scanned, and we examine the query to
determine which ones can be pruned.  b) by default, no partitions are
scanned, and we examine the query to determine which ones must be
scanned.

The whole "enable_partition_pruning" thing is based on the idea that we
do a).  You propose that we do b) instead.  The only difference is what
happens if the feature is disabled -- the "by default" clause gets
inverted.  So it would have to be b) if the feature is enabled, by
default no partitions are scanned, and we examine the query to determine
which ones must be scanned; if the feature is disabled, all partitions
must be scanned.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

David G. Johnston wrote:

> As a user I don't really need to know which model is implemented and the
> name doesn't necessarily imply the implementation.  Pruning seems to be the
> commonly-used term for this feature and we should stick with that.

I agree with this conclusion.  So we have it right and we shouldn't
change it.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, May 10, 2018 at 12:58 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> David G. Johnston wrote:
>> As a user I don't really need to know which model is implemented and the
>> name doesn't necessarily imply the implementation.  Pruning seems to be the
>> commonly-used term for this feature and we should stick with that.
>
> I agree with this conclusion.  So we have it right and we shouldn't
> change it.

+1.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

David G. Johnston wrote:

> ​Seems like if it stays the name is good - but at this point no has voiced
> opposition to removing it and making the name a moot point.

If we think the probability of bugs is 0%, then I'm all for removing it.
I don't.  I vote to remove the GUC in a couple of releases, once it's
proven completely useless.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, May 10, 2018 at 1:51 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> David G. Johnston wrote:
>> Seems like if it stays the name is good - but at this point no has voiced
>> opposition to removing it and making the name a moot point.
>
> If we think the probability of bugs is 0%, then I'm all for removing it.
> I don't.  I vote to remove the GUC in a couple of releases, once it's
> proven completely useless.

No feature ever written has a 0% probability of bugs.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

I'm thinking something like this.

The examples for runtime pruning are lame -- in the first, the text says
"watch out for Subplans Removed" and then the example provided doesn't
show one.  (That example is probably exercising the wrong thing.)

Anyway, wording suggestions for 5.10.4 and 5.10.5 in the attached file
are welcome.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, May 10, 2018 at 3:45 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> The examples for runtime pruning are lame -- in the first, the text says
> "watch out for Subplans Removed" and then the example provided doesn't
> show one.  (That example is probably exercising the wrong thing.)

It seems to me that EXPLAIN output should have a clear way to show --
and to distinguish -- (1) plan-time pruning, (2) executor startup time
pruning, (3) mid-execution pruning.  I don't think that's entirely the
case right now.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 11 May 2018 at 08:05, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Thu, May 10, 2018 at 3:45 PM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
> > The examples for runtime pruning are lame -- in the first, the text says
> > "watch out for Subplans Removed" and then the example provided doesn't
> > show one.  (That example is probably exercising the wrong thing.)
>
> It seems to me that EXPLAIN output should have a clear way to show --
> and to distinguish -- (1) plan-time pruning, (2) executor startup time
> pruning, (3) mid-execution pruning.  I don't think that's entirely the
> case right now.

I'm open to improving this, but I've just not come up with any bright
ideas on how to, yet.

Here's a recap of the current way to determine where the pruning occurred:

Phase 1: Plan time pruning:

EXPLAIN/EXPLAIN ANALYZE shows Append/MergeAppend/ModifyTable shows
fewer subnodes than there are partitions.
Both EXPLAIN and EXPLAIN ANALYZE output gives no indication of any pruning.

Phase 2: Executor init pruning:

EXPLAIN and EXPLAIN ANALYZE shows Append with fewer subnodes than
there are partitions + "Subplans Removed: <N>" appears to indicate the
number of subnodes removed by this phase.

MergeAppend and ModifyTable are unsupported in PG11.

Phase 3: Executor run pruning:

EXPLAIN/EXPLAIN ANALYZE shows all nodes that survived phase 1+2.

EXPLAIN ANALYZE shows that if a given node was never executed then the
runtime times appear as "(never executed)". If the Append was executed
and a subnode the Append was "(never executed)" then it was pruned by
this phase.

Changing parameters may cause some nodes to be scanned fewer times
than other nodes. The "nloops" count being lower than the nloop count
of the Append indicates this. e.g nloops=5 on an Append subnode vs
nloops=8 on the Append node indicates the node was eliminated 3 times.

Although complications around Parallel Append could make it quite
difficult to count nloops, since a node running a partial plan could
be executed by may workers which would increase the nloops.


Solutions?

The best I can think of right now is to add 2 more int properties to
the EXPLAIN output:

1. Subplans removed by plan-time constraints exclusion: N
2. Subplans removed by plan-time partition pruning: N

The rename the "Subplans Removed" that's there today to "Subplans
removed by run-time pruning"

These names are not very good, also. I'm also not very excited about
adding this. This also does nothing for phase 3.

Would something like that address your concern?  Or do you have another idea?

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

On 2018/05/11 2:13, Robert Haas wrote:
> On Thu, May 10, 2018 at 12:58 PM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>> David G. Johnston wrote:
>>> As a user I don't really need to know which model is implemented and the
>>> name doesn't necessarily imply the implementation.  Pruning seems to be the
>>> commonly-used term for this feature and we should stick with that.
>>
>> I agree with this conclusion.  So we have it right and we shouldn't
>> change it.
> 
> +1.

+1 from me too.

Thanks,
Amit

Hi.

On 2018/05/11 4:45, Alvaro Herrera wrote:
> I'm thinking something like this.

+1 to this more radical overhaul of this part of the documentation.

> The examples for runtime pruning are lame -- in the first, the text says
> "watch out for Subplans Removed" and then the example provided doesn't
> show one.  (That example is probably exercising the wrong thing.)
> 
> Anyway, wording suggestions for 5.10.4 and 5.10.5 in the attached file
> are welcome.

A few comments.

1. At the beginning of 5.10.4, in this example EXPLAIN's output:

SET enable_partition_pruning = off;
EXPLAIN SELECT count(*) FROM measurement WHERE logdate >= DATE '2008-01-01';

There used to be [1] ellipses to show discontinuation between partitions
shown in the output plan, which no longer exists.  Should be like this:

->  Seq Scan on measurement_y2006m03  (cost=0.00..33.12 rows=617 width=0)
    Filter: (logdate >= '2008-01-01'::date)
...
->  Seq Scan on measurement_y2007m11  (cost=0.00..33.12 rows=617 width=0)
    Filter: (logdate >= '2008-01-01'::date)

2. In the following sentence in 5.10.5

"Constraint exclusion works in a very similar way to partition pruning,
except that it uses each table's CHECK constraints — which gives it its
name — instead of the partitioning constraints, as with partition pruning.
Another difference is that it is only applied at plan time; there is no
attempt to remove partitions at execution time."

I think that saying "instead of the partitioning constraints, as with
partition pruning" here may be a bit misleading, because it may give
readers an impression that *all* tables have a partitioning constraint but
constraint exclusion ignores it in favor of using CHECK constraints.  How
about saying:

whereas partition pruning uses a table's partitioning constraint which
exists only in the case of declarative partitioning.

3. Do we want the following sentence 5.10.5 to be revised now?

"The default (and recommended) setting of constraint_exclusion is actually
neither on nor off, but an intermediate setting called partition, which
causes the technique to be applied only to queries that are likely to be
working on inheritance partitioned tables."

I'm not sure if it's the time yet, but maybe we would want to recommend
"on" and mention that users may want to switch to "partition" if they need
to use legacy inheritance partitioning for one reason or another.

4. In the following sentence in the caveats part of 5.10.5. Partitioning
and Constraint Exclusion

"A good rule of thumb is that partitioning constraints should contain only
comparisons of the partitioning column(s) to constants using
B-tree-indexable operators, which applies even to partitioned tables,
because only B-tree-indexable column(s) are allowed in the partition key."

The part beginning with ", which applies even to partitioned tables" is no
longer needed as I had pointed out upthread [2].  The reason is we no
longer pass the partition key derived partition constraints to constraint
exclusion algorithm, as the new pruning covers that base.

5. The last sentence in caveats, that is,

"Partitioning using these techniques will work well with up to perhaps a
hundred partitions; don't try to use many thousands of partitions."

should perhaps be reworded as:

"So the legacy inheritance based partitioning will work well with up to
perhaps a hundred partitions; don't try to use many thousands of partitions."

Thanks,
Amit

[1] https://www.postgresql.org/docs/10/static/ddl-partitioning.html

[2]
https://www.postgresql.org/message-id/a8ad3dd8-ef30-bbd0-6732-a673710378fa%40lab.ntt.co.jp

On Fri, May 11, 2018 at 12:59:27PM +0900, Amit Langote wrote:
> On 2018/05/11 2:13, Robert Haas wrote:
>> On Thu, May 10, 2018 at 12:58 PM, Alvaro Herrera
>> <alvherre@2ndquadrant.com> wrote:
>>> David G. Johnston wrote:
>>>> As a user I don't really need to know which model is implemented and the
>>>> name doesn't necessarily imply the implementation.  Pruning seems to be the
>>>> commonly-used term for this feature and we should stick with that.
>>>
>>> I agree with this conclusion.  So we have it right and we shouldn't
>>> change it.
>>
>> +1.
>
> +1 from me too.

+1.
--
Michael

Hello

Amit Langote wrote:

> +1 to this more radical overhaul of this part of the documentation.

Thanks.  I pushed now after some more tweaking, including your suggested
corrections.  I removed the examples, because they were both wrong.  We
can give this more polish if anybody has the energy, but I think we're
in a pretty decent place now.

I'm not convinced that we need to show so much detail on pruning as
proposed by Robert elsewhere; we didn't have a lot of detail for
exclusion either and I don't have any evidence that it was a terrible
problem for users.  Also, one possible use of the new GUC is that you
can compare plans if you so wish.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 13 May 2018 at 03:30, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Amit Langote wrote:
>
>> +1 to this more radical overhaul of this part of the documentation.
>
> Thanks.  I pushed now after some more tweaking,

Thanks for pushing.

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

On 2018/05/14 9:55, David Rowley wrote:
> On 13 May 2018 at 03:30, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>> Amit Langote wrote:
>>
>>> +1 to this more radical overhaul of this part of the documentation.
>>
>> Thanks.  I pushed now after some more tweaking,
> 
> Thanks for pushing.

Thank you.

Regards,
Amit

On Thu, May 10, 2018 at 10:22 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
> Here's a recap of the current way to determine where the pruning occurred:
>
> Phase 1: Plan time pruning:
>
> EXPLAIN/EXPLAIN ANALYZE shows Append/MergeAppend/ModifyTable shows
> fewer subnodes than there are partitions.
> Both EXPLAIN and EXPLAIN ANALYZE output gives no indication of any pruning.
>
> Phase 2: Executor init pruning:
>
> EXPLAIN and EXPLAIN ANALYZE shows Append with fewer subnodes than
> there are partitions + "Subplans Removed: <N>" appears to indicate the
> number of subnodes removed by this phase.
>
> MergeAppend and ModifyTable are unsupported in PG11.
>
> Phase 3: Executor run pruning:
>
> EXPLAIN/EXPLAIN ANALYZE shows all nodes that survived phase 1+2.
>
> EXPLAIN ANALYZE shows that if a given node was never executed then the
> runtime times appear as "(never executed)". If the Append was executed
> and a subnode the Append was "(never executed)" then it was pruned by
> this phase.

Hmm, that's actually not as bad as I thought.  Thanks for the
explanation.  I think if I were going to try to improve things, I'd
try to annotate the Append node with the name of the partitioned table
that it's using for pruning in case #2 and case #3, and maybe
something to indicate which type of pruning is in use.  That would
make it really clear whether pruning is enabled or not.  The methods
you mention above sort of require reading the tea leaves -- and it
might not always be very easy to distinguish between cases where
pruning is possible but nothing got pruned (imagine an inequality
qual) and where it's not even possible in the first place.

e.g.

Append
  Execution-Time Pruning: order_lines (at executor startup)
  -> Index Scan ...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 17 May 2018 at 01:19, Robert Haas <robertmhaas@gmail.com> wrote:
> Hmm, that's actually not as bad as I thought.  Thanks for the
> explanation.  I think if I were going to try to improve things, I'd
> try to annotate the Append node with the name of the partitioned table
> that it's using for pruning in case #2 and case #3, and maybe
> something to indicate which type of pruning is in use.  That would
> make it really clear whether pruning is enabled or not.  The methods
> you mention above sort of require reading the tea leaves -- and it
> might not always be very easy to distinguish between cases where
> pruning is possible but nothing got pruned (imagine an inequality
> qual) and where it's not even possible in the first place.
>
> e.g.
>
> Append
>   Execution-Time Pruning: order_lines (at executor startup)
>   -> Index Scan ...

Perhaps Append should be shown as "Unordered Partitioned Table Scan on
<table>". That seems more aligned to how else we show which relation a
node belongs to. The partition being scanned is simple to obtain. It's
just the first item in the partitioned_rels List. (MergeAppend would
be an "Ordered Partitioned Table Scan")

I'm not really a fan of overloading properties with a bunch of text.
Multiple int or text properties would be easier to deal with,
especially so when you consider the other explain formats. Remember,
all 3 pruning methods could be used for a single Append node.

I guess doing work here would require additional code in the planner
to track how many relations were removed by both partition pruning and
constraint exclusion. Dunno if that would be tracked together or
separately. However, I'd prefer to have a clear idea of what exactly
the design should be before I go write some code that perhaps nobody
will like.

Unsure what you have in mind for the pruning done during actual
execution; just a yay or nay as to whether we're attempting it or not?

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

On Thu, May 17, 2018 at 12:04 AM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
>> Append
>>   Execution-Time Pruning: order_lines (at executor startup)
>>   -> Index Scan ...
>
> Perhaps Append should be shown as "Unordered Partitioned Table Scan on
> <table>". That seems more aligned to how else we show which relation a
> node belongs to. The partition being scanned is simple to obtain. It's
> just the first item in the partitioned_rels List. (MergeAppend would
> be an "Ordered Partitioned Table Scan")

Hmm, that's a radical proposal but I'm not sure I like it.  For one
thing, table scan might mean sequential scan to some users.  For
another, it's not really unordered.  Unless it's parallel-aware, we're
going to scan them strictly in the order they're given.

> I'm not really a fan of overloading properties with a bunch of text.
> Multiple int or text properties would be easier to deal with,
> especially so when you consider the other explain formats. Remember,
> all 3 pruning methods could be used for a single Append node.

I was imagining it as two properties in non-text format that got
displayed in a special way in text mode.  I intended that this would
only give information about execution-time pruning, so there would
only two methods to consider here, but, yeah, you might have something
like:

Execution-Time Pruning: order_lines (at executor startup, at runtime)

> I guess doing work here would require additional code in the planner
> to track how many relations were removed by both partition pruning and
> constraint exclusion. Dunno if that would be tracked together or
> separately. However, I'd prefer to have a clear idea of what exactly
> the design should be before I go write some code that perhaps nobody
> will like.

I don't feel strongly about adding more code to track the number of
removed partitions.  I think that the important thing is whether or
not partitioning is happening and at what stage, and I think it's
useful to show the relation name if we can.  As you pointed out, it's
largely possible already to figure out how well we did at removing
stuff and at which stages, but to me it seems quite easy to be
confused about which stages tried to remove things.  For example,
consider:

Gather
-> Nested Loop
  -> Seq Scan
    Filter: something
  -> Append
    -> Index Scan
    -> Index Scan
    -> Index Scan

I think it's going to be quite tricky to figure out whether that
Append node is trying to do execution-time pruning without some
annotation.  The nloops values are going to be affected by how many
rows are in which partitions and how many workers got which rows as
well as by whether execution-time pruning worked and how effectively.
You might be able to figure out it out by staring at the EXPLAIN
output for a while... but it sure seems like it would be a lot nicer
to have an explicit indicator... especially if you're some random user
rather than a PostgreSQL expect.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2018/05/17 23:24, Robert Haas wrote:
> On Thu, May 17, 2018 at 12:04 AM, David Rowley wrote:
>> I'm not really a fan of overloading properties with a bunch of text.
>> Multiple int or text properties would be easier to deal with,
>> especially so when you consider the other explain formats. Remember,
>> all 3 pruning methods could be used for a single Append node.
> 
> I was imagining it as two properties in non-text format that got
> displayed in a special way in text mode.  I intended that this would
> only give information about execution-time pruning, so there would
> only two methods to consider here, but, yeah, you might have something
> like:
> 
> Execution-Time Pruning: order_lines (at executor startup, at runtime)

This looks short enough and useful.

>> I guess doing work here would require additional code in the planner
>> to track how many relations were removed by both partition pruning and
>> constraint exclusion. Dunno if that would be tracked together or
>> separately. However, I'd prefer to have a clear idea of what exactly
>> the design should be before I go write some code that perhaps nobody
>> will like.
> 
> I don't feel strongly about adding more code to track the number of
> removed partitions.  I think that the important thing is whether or
> not partitioning is happening and at what stage, and I think it's
> useful to show the relation name if we can.  As you pointed out, it's
> largely possible already to figure out how well we did at removing
> stuff and at which stages, but to me it seems quite easy to be
> confused about which stages tried to remove things.  For example,
> consider:
> 
> Gather
> -> Nested Loop
>   -> Seq Scan
>     Filter: something
>   -> Append
>     -> Index Scan
>     -> Index Scan
>     -> Index Scan
> 
> I think it's going to be quite tricky to figure out whether that
> Append node is trying to do execution-time pruning without some
> annotation.  The nloops values are going to be affected by how many
> rows are in which partitions and how many workers got which rows as
> well as by whether execution-time pruning worked and how effectively.
> You might be able to figure out it out by staring at the EXPLAIN
> output for a while... but it sure seems like it would be a lot nicer
> to have an explicit indicator... especially if you're some random user
> rather than a PostgreSQL expect.

Yeah, I think it'd help to have Append be annotated as suggested by Robert
above.  I guess if "at executor startup" is shown, then the subnodes
listed under Append will consist of only those that survived
executor-startup pruning and thus will help understand why there are fewer
than shown with EXPLAIN (without ANALYZE).  Also, if "at runtime" is
shown, a user may want look at nloops property of the individual subnodes
to guess at how much pruning has occurred; although only the latter (that
is, inspecting nloops) suffices to know that runtime pruning has occurred
as also currently written in the documentation about pruning [1], the
first piece of information (the "at runtime" annotation) seems nice to have.

Thanks,
Amit

[1]
https://www.postgresql.org/docs/devel/static/ddl-partitioning.html#DDL-PARTITION-PRUNING

On Fri, May 18, 2018 at 4:22 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Yeah, I think it'd help to have Append be annotated as suggested by Robert
> above.  I guess if "at executor startup" is shown, then the subnodes
> listed under Append will consist of only those that survived
> executor-startup pruning and thus will help understand why there are fewer
> than shown with EXPLAIN (without ANALYZE).  Also, if "at runtime" is
> shown, a user may want look at nloops property of the individual subnodes
> to guess at how much pruning has occurred; although only the latter (that
> is, inspecting nloops) suffices to know that runtime pruning has occurred
> as also currently written in the documentation about pruning [1], the
> first piece of information (the "at runtime" annotation) seems nice to have.

Having EXPLAIN and EXPLAIN ANALYZE show different things doesn't sound
like a good idea.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

I reread this and have some more comments.
https://www.postgresql.org/docs/devel/static/ddl-partitioning.html

"however, it is not possible to use some of the inheritance features discussed
in the previous section with partitioned tables and partitions"

=> The referenced section now follows rather than precedes the text; I suggest
to say:
"however, it is not possible to use some features of inheritance (discussed
below) with declaratively partitioned tables or their partitions"

"It is neither possible to specify columns when creating partitions with CREATE
TABLE nor is it possible to add columns to partitions after-the-fact using
ALTER TABLE"
=> change to: "It is not possible .. nor is it possible .."

Immediately after the section in inheritence:
https://www.postgresql.org/docs/devel/static/ddl-partitioning.html#DDL-PARTITION-PRUNING
"Partition pruning is a query optimization technique that improves performance
for partitioned tables"
=> I think should say "improves performance for DECLARATIVELY partitioned
tables"

"You can use the EXPLAIN command to show the difference between a plan whose
partitions have been pruned from one whose partitions haven't, by using the
enable_partition_pruning configuration parameter. A typical unoptimized plan
for this type of table setup is:"
=> should say "difference between .. AND", not FROM.

=> Also, should avoid repeating "use...using".  Also, remove the comma or
rearrange the sentence:
"By using the EXPLAIN command and the enable_partition_pruning configuration
parameter, one can show the difference between a plan whose partitions have
been pruned from one whose partitions haven't.

"Constraint exclusion is only applied during query planning; it is not applied
at execution time like partition pruning does."

=> Remove "does" ?

"Partitioning enforces a rule that all partitions must have exactly the same
set of columns as the parent"

=> I think should say "Declarative partitioning enforces"; or maybe:
"Partitions of a partitioned table must have exactly the same set of columns as
the parent"
or:
"For declarative partitioning, partitions must have exactly the same set of
columns as the partitioned table"

Let me know if it's useful to provide a patch.

Justin

On Sat, May 19, 2018 at 5:02 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, May 18, 2018 at 4:22 AM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> Yeah, I think it'd help to have Append be annotated as suggested by Robert
>> above.  I guess if "at executor startup" is shown, then the subnodes
>> listed under Append will consist of only those that survived
>> executor-startup pruning and thus will help understand why there are fewer
>> than shown with EXPLAIN (without ANALYZE).  Also, if "at runtime" is
>> shown, a user may want look at nloops property of the individual subnodes
>> to guess at how much pruning has occurred; although only the latter (that
>> is, inspecting nloops) suffices to know that runtime pruning has occurred
>> as also currently written in the documentation about pruning [1], the
>> first piece of information (the "at runtime" annotation) seems nice to have.
>
> Having EXPLAIN and EXPLAIN ANALYZE show different things doesn't sound
> like a good idea.

Hmm yeah.  I think I was misunderstanding how executor-startup pruning
works when I wrote:

...and thus will help understand why there are fewer
than shown with EXPLAIN (without ANALYZE).

Actually, because ExecInitAppend would run for both EXPLAIN and
EXPLAIN ANALYZE, executor-startup pruning should occur in both cases
and will result in the same plan shape to be shown.  Sorry about the
confusion.

Thanks,
Amit

On Fri, May 18, 2018 at 08:56:53PM -0500, Justin Pryzby wrote:
> I reread this and have some more comments.
> https://www.postgresql.org/docs/devel/static/ddl-partitioning.html

> Let me know if it's useful to provide a patch.

I propose this.

There's two other, wider changes to consider:

 - should "5.10.4. Partition Pruning" be moved after "5.10.2. Declarative
   Partitioning", rather than after "5.10.3. Implementation Using Inheritance" ?
 - should we find a unified term for "inheritence-based partitioning" and avoid
   using the word "partitioning" in that context?  For example: "Partitioning
   can be implemented using table inheritance[...]".  One possible phrase
   currently begin used is: "legacy inheritance method".

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 2cd0b8a..6e1ade9 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -2967,70 +2967,70 @@ VALUES ('Albany', NULL, NULL, 'NY');
    </para>
 
    <para>
     It is not possible to turn a regular table into a partitioned table or
     vice versa.  However, it is possible to add a regular or partitioned table
     containing data as a partition of a partitioned table, or remove a
     partition from a partitioned table turning it into a standalone table;
     see <xref linkend="sql-altertable"/> to learn more about the
     <command>ATTACH PARTITION</command> and <command>DETACH PARTITION</command>
     sub-commands.
    </para>
 
    <para>
     Individual partitions are linked to the partitioned table with inheritance
-    behind-the-scenes; however, it is not possible to use some of the
-    inheritance features discussed in the previous section with partitioned
-    tables and partitions.  For example, a partition cannot have any parents
+    behind-the-scenes; however, it is not possible to use some of the generic
+    features of inheritance (discussed below) with declaratively partitioned
+    tables or their partitions For example, a partition cannot have any parents
     other than the partitioned table it is a partition of, nor can a regular
-    table inherit from a partitioned table making the latter its parent.
-    That means partitioned tables and partitions do not participate in
-    inheritance with regular tables.  Since a partition hierarchy consisting
-    of the partitioned table and its partitions is still an inheritance
-    hierarchy, all the normal rules of inheritance apply as described in
+    table inherit from a partitioned table making the latter its parent.  That
+    means partitioned tables and partitions do not participate in inheritance
+    with regular tables.  Since a partition hierarchy consisting of the
+    partitioned table and its partitions is still an inheritance hierarchy, all
+    the normal rules of inheritance apply as described in
     <xref linkend="ddl-inherit"/> with some exceptions, most notably:
 
     <itemizedlist>
      <listitem>
       <para>
        Both <literal>CHECK</literal> and <literal>NOT NULL</literal>
        constraints of a partitioned table are always inherited by all its
        partitions.  <literal>CHECK</literal> constraints that are marked
        <literal>NO INHERIT</literal> are not allowed to be created on
        partitioned tables.
       </para>
      </listitem>
 
      <listitem>
       <para>
        Using <literal>ONLY</literal> to add or drop a constraint on only the
        partitioned table is supported when there are no partitions.  Once
        partitions exist, using <literal>ONLY</literal> will result in an error
        as adding or dropping constraints on only the partitioned table, when
-       partitions exist, is not supported.  Instead, constraints can be added
-       or dropped, when they are not present in the parent table, directly on
-       the partitions.  As a partitioned table does not have any data
-       directly, attempts to use <command>TRUNCATE</command>
+       partitions exist, is not supported.  Instead, constraints on the
+       partitions themselves can be added and (if they are not present in the
+       parent table) dropped.  As a partitioned table does not
+       have any data directly, attempts to use <command>TRUNCATE</command>
        <literal>ONLY</literal> on a partitioned table will always return an
        error.
       </para>
      </listitem>
 
      <listitem>
       <para>
        Partitions cannot have columns that are not present in the parent.  It
-       is neither possible to specify columns when creating partitions with
-       <command>CREATE TABLE</command> nor is it possible to add columns to
+       is not possible to specify columns when creating partitions with
+       <command>CREATE TABLE</command>, nor is it possible to add columns to
        partitions after-the-fact using <command>ALTER TABLE</command>.  Tables may be
        added as a partition with <command>ALTER TABLE ... ATTACH PARTITION</command>
        only if their columns exactly match the parent, including any
        <literal>oid</literal> column.
       </para>
      </listitem>
 
      <listitem>
       <para>
        You cannot drop the <literal>NOT NULL</literal> constraint on a
        partition's column if the constraint is present in the parent table.
       </para>
      </listitem>
     </itemizedlist>
@@ -3347,37 +3347,37 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
        on individual partitions, not the partitioned table.
       </para>
      </listitem>
     </itemizedlist>
     </para>
     </sect3>
    </sect2>
 
    <sect2 id="ddl-partitioning-implementation-inheritance">
     <title>Implementation Using Inheritance</title>
     <para>
      While the built-in declarative partitioning is suitable for most
      common use cases, there are some circumstances where a more flexible
      approach may be useful.  Partitioning can be implemented using table
-     inheritance, which allows for several features which are not supported
+     inheritance, which allows for several features not supported
      by declarative partitioning, such as:
 
      <itemizedlist>
       <listitem>
        <para>
-        Partitioning enforces a rule that all partitions must have exactly
-        the same set of columns as the parent, but table inheritance allows
-        children to have extra columns not present in the parent.
+    For declarative partitioning, partitions must have exactly the same set
+    of columns as the partitioned table, whereas with table inheritance,
+    child tables may have extra columns not present in the parent.
        </para>
       </listitem>
 
       <listitem>
        <para>
         Table inheritance allows for multiple inheritance.
        </para>
       </listitem>
 
       <listitem>
        <para>
         Declarative partitioning only supports range, list and hash
         partitioning, whereas table inheritance allows data to be divided in a
         manner of the user's choosing.  (Note, however, that if constraint
@@ -3757,52 +3757,51 @@ ANALYZE measurement;
      </itemizedlist>
     </para>
    </sect3>
   </sect2>
 
   <sect2 id="ddl-partition-pruning">
    <title>Partition Pruning</title>
 
    <indexterm>
     <primary>partition pruning</primary>
    </indexterm>
 
    <para>
     <firstterm>Partition pruning</firstterm> is a query optimization technique
-    that improves performance for partitioned tables.  As an example:
+    that improves performance for declaratively partitioned tables.  As an example:
 
 <programlisting>
 SET enable_partition_pruning = on;                 -- the default
 SELECT count(*) FROM measurement WHERE logdate >= DATE '2008-01-01';
 </programlisting>
 
     Without partition pruning, the above query would scan each of the
     partitions of the <structname>measurement</structname> table. With
     partition pruning enabled, the planner will examine the definition
     of each partition and prove that the partition need not
     be scanned because it could not contain any rows meeting the query's
     <literal>WHERE</literal> clause.  When the planner can prove this, it
     excludes (<firstterm>prunes</firstterm>) the partition from the query
     plan.
    </para>
 
    <para>
-    You can use the <command>EXPLAIN</command> command to show the
-    difference between a plan whose partitions have been pruned from one
-    whose partitions haven't, by using the
-    <xref linkend="guc-enable-partition-pruning"/> configuration
-    parameter.  A typical unoptimized plan for this type of table setup
-    is:
+    By using the EXPLAIN command and the <xref
+    linkend="guc-enable-partition-pruning"/> configuration parameter, it's
+    possible to show the difference between a plan whose partitions have been
+    pruned and one whose partitions haven't.  A typical unoptimized plan for
+    this type of table setup is:
 <programlisting>
 SET enable_partition_pruning = off;
 EXPLAIN SELECT count(*) FROM measurement WHERE logdate >= DATE '2008-01-01';
                                     QUERY PLAN
 ───────────────────────────────────────────────────────────────────────────────────
  Aggregate  (cost=188.76..188.77 rows=1 width=8)
    ->  Append  (cost=0.00..181.05 rows=3085 width=0)
          ->  Seq Scan on measurement_y2006m02  (cost=0.00..33.12 rows=617 width=0)
                Filter: (logdate >= '2008-01-01'::date)
          ->  Seq Scan on measurement_y2006m03  (cost=0.00..33.12 rows=617 width=0)
                Filter: (logdate >= '2008-01-01'::date)
 ...
          ->  Seq Scan on measurement_y2007m11  (cost=0.00..33.12 rows=617 width=0)
                Filter: (logdate >= '2008-01-01'::date)
@@ -3881,31 +3880,31 @@ EXPLAIN SELECT count(*) FROM measurement WHERE logdate >= DATE '2008-01-01';
      </listitem>
     </itemizedlist>
    </para>
 
    <para>
     Partition pruning can be disabled using the
     <xref linkend="guc-enable-partition-pruning"/> setting.
    </para>
 
    <note>
     <para>
      Currently, pruning of partitions during the planning of an
      <command>UPDATE</command> or <command>DELETE</command> command is
      implemented using the constraint exclusion method (however, it is
-     still ruled by the <literal>enable_partition_pruning</literal>
-     setting instead of <literal>constraint_exclusion</literal>) —
-     see the next section for details and caveats that apply.
+     controlled ruled by the <literal>enable_partition_pruning</literal>
+     rather than <literal>constraint_exclusion</literal>) —
+     see the following section for details and caveats that apply.
     </para>
 
     <para>
      Also, execution-time partition pruning currently only occurs for the
      <literal>Append</literal> node type, not <literal>MergeAppend</literal>.
     </para>
 
     <para>
      Both of these behaviors are likely to be changed in a future release
      of <productname>PostgreSQL</productname>.
     </para>
    </note>
   </sect2>
 
@@ -3916,72 +3915,72 @@ EXPLAIN SELECT count(*) FROM measurement WHERE logdate >= DATE '2008-01-01';
     <primary>constraint exclusion</primary>
    </indexterm>
 
    <para>
     <firstterm>Constraint exclusion</firstterm> is a query optimization
     technique similar to partition pruning.  While it is primarily used
     for partitioned tables using the legacy inheritance method, it can be
     used for other purposes, including with declarative partitioning.
    </para>
 
    <para>
     Constraint exclusion works in a very similar way to partition
     pruning, except that it uses each table's <literal>CHECK</literal>
     constraints — which gives it its name — whereas partition
-    pruning uses the table's partitioning constraint, which exists only in
-    the case of declarative partitioning.  Another difference is that it
-    is only applied at plan time; there is no attempt to remove
-    partitions at execution time.
+    pruning uses the table's partitioning bounds, which exists only in
+    the case of declarative partitioning.  Another difference is that
+    constraint exclusion is only applied at plan time; there is no attempt to
+    remove partitions at execution time.
    </para>
 
    <para>
     The fact that constraint exclusion uses <literal>CHECK</literal>
     constraints, which makes it slow compared to partition pruning, can
     sometimes be used as an advantage: because constraints can be defined
     even on declaratively-partitioned tables, in addition to the internal
     partitioning constraints, and only constraint exclusion would be able
     to elide certain partitions from the query plan using those.
    </para>
 
    <para>
     The default (and recommended) setting of
     <xref linkend="guc-constraint-exclusion"/> is neither
     <literal>on</literal> nor <literal>off</literal>, but an intermediate setting
     called <literal>partition</literal>, which causes the technique to be
     applied only to queries that are likely to be working on inheritance partitioned
     tables.  The <literal>on</literal> setting causes the planner to examine
     <literal>CHECK</literal> constraints in all queries, even simple ones that
     are unlikely to benefit.
    </para>
 
    <para>
     The following caveats apply to constraint exclusion:
 
    <itemizedlist>
     <listitem>
      <para>
-      Constraint exclusion is only applied during query planning; it is
-      not applied at execution time like partition pruning does.
+      Constraint exclusion is only applied during query planning; unlike
+      partition pruning, it cannot be not applied during execution.
      </para>
     </listitem>
 
     <listitem>
      <para>
       Constraint exclusion only works when the query's <literal>WHERE</literal>
       clause contains constants (or externally supplied parameters).
       For example, a comparison against a non-immutable function such as
       <function>CURRENT_TIMESTAMP</function> cannot be optimized, since the
-      planner cannot know which partition the function value might fall
+      planner cannot know which partition the function's value might fall
       into at run time.
      </para>
     </listitem>
 
     <listitem>
      <para>
       Keep the partitioning constraints simple, else the planner may not be
       able to prove that partitions don't need to be visited.  Use simple
       equality conditions for list partitioning, or simple
       range tests for range partitioning, as illustrated in the preceding
       examples.  A good rule of thumb is that partitioning constraints should
       contain only comparisons of the partitioning column(s) to constants
       using B-tree-indexable operators, because only B-tree-indexable
       column(s) are allowed in the partition key.

On 24 May 2018 at 09:35, Justin Pryzby <pryzby@telsasoft.com> wrote:
> On Fri, May 18, 2018 at 08:56:53PM -0500, Justin Pryzby wrote:
>> I reread this and have some more comments.
>> https://www.postgresql.org/docs/devel/static/ddl-partitioning.html
>
>> Let me know if it's useful to provide a patch.
>
> I propose this.

Thanks for working on this.

Can you just attach the patch?

Personally, for me, it's much easier to review when applied rather
than looking at an email.

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

On Thu, May 24, 2018 at 10:46:38AM +1200, David Rowley wrote:
> On 24 May 2018 at 09:35, Justin Pryzby <pryzby@telsasoft.com> wrote:
> > On Fri, May 18, 2018 at 08:56:53PM -0500, Justin Pryzby wrote:
> >> I reread this and have some more comments.
> >> https://www.postgresql.org/docs/devel/static/ddl-partitioning.html
> >
> >> Let me know if it's useful to provide a patch.
> >
> > I propose this.
> 
> Thanks for working on this.
> 
> Can you just attach the patch?

Attached.

Justin

Hi Justin.

Thanks for writing the patch.  I have a couple of comments.

On 2018/05/24 8:31, Justin Pryzby wrote:
> On Thu, May 24, 2018 at 10:46:38AM +1200, David Rowley wrote:
>> On 24 May 2018 at 09:35, Justin Pryzby <pryzby@telsasoft.com> wrote:
>>> On Fri, May 18, 2018 at 08:56:53PM -0500, Justin Pryzby wrote:
>>>> I reread this and have some more comments.
>>>> https://www.postgresql.org/docs/devel/static/ddl-partitioning.html
>>>
>>>> Let me know if it's useful to provide a patch.
>>>
>>> I propose this.
>>
>> Thanks for working on this.
>>
>> Can you just attach the patch?
> 
> Attached.

-    behind-the-scenes; however, it is not possible to use some of the
-    inheritance features discussed in the previous section with partitioned
-    tables and partitions.  For example, a partition cannot have any parents
+    behind-the-scenes; however, it is not possible to use some of the generic
+    features of inheritance (discussed below) with declaratively partitioned
+    tables or their partitions For example, a partition cannot have any
parents

As I recall, I had written the "previous section" in the original text to
mean 5.9 Inheritance

https://www.postgresql.org/docs/devel/static/ddl-inherit.html

Although, we do list some inheritance features that cannot be used with
declarative partitioned tables on the same page in 5.10.3, so what you
have here may be fine.

+    possible to show the difference between a plan whose partitions have been
+    pruned and one whose partitions haven't.  A typical unoptimized plan for
+    this type of table setup is:

"a plan whose partitions have been pruned" sounds a bit off; maybe, "a
plan in which partitions have been pruned".

+     controlled ruled by the <literal>enable_partition_pruning</literal>

controlled ruled by -> still controlled by

-    pruning uses the table's partitioning constraint, which exists only in
-    the case of declarative partitioning.
...
+    pruning uses the table's partitioning bounds, which exists only in
+    the case of declarative partitioning.

Maybe say "partition bounds" here if change it at all.

Thanks,
Amit

On Thu, May 24, 2018 at 11:30:40AM +0900, Amit Langote wrote:
> Hi Justin.
> 
> Thanks for writing the patch.  I have a couple of comments.

Thanks for your review, find attached updated patch.

> +    possible to show the difference between a plan whose partitions have been
> +    pruned and one whose partitions haven't.  A typical unoptimized plan for
> +    this type of table setup is:
> 
> "a plan whose partitions have been pruned" sounds a bit off; maybe, "a
> plan in which partitions have been pruned".

I wrote:

"[...] a plan for which partitions have been pruned and for which they have
not."

Cheers,
Justin

On Wed, May  2, 2018 at 07:49:42PM -0700, David G. Johnston wrote:
> On Wednesday, May 2, 2018, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> 
>     Robert Haas wrote:
>     > On Wed, May 2, 2018 at 9:28 AM, Alvaro Herrera <alvherre@2ndquadrant.com>
>     wrote:
>     > > I admit I am more concerned about the possibility of bugs than I am
>     > > about providing a performance-related tool.
>     >
>     > I agree that if partition pruning has bugs, somebody might want to
>     > turn it off.  On the other hand, when they do, there's a good chance
>     > that they will lose so much performance that they'll still be pretty
>     > sad.  Somebody certainly could have a workload where the pruning
>     > helps, but by a small enough amount that shutting it off is
>     > acceptable.  But I suspect that's a somewhat narrow target.
>     >
>     > I'm not going to go to war over this, though.  I'm just telling you
>     > what I think.
> 
>     Well, we didn't have a GUC initially, evidently because none of us
>     thought that this would be a huge problem.  So maybe you're both right
>     and it's overkill to have it.  I'm not set on having it, either.  Does
>     anybody else have an opinion?
> 
> 
> I toss my +1 to removing it altogether.

+1  We are terrible at removing old GUCs and having it around means
everyone has to decide if they need to change it, so having it is not a
zero cost.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

On 2018-May-24, Bruce Momjian wrote:

> On Wed, May  2, 2018 at 07:49:42PM -0700, David G. Johnston wrote:

> > I toss my +1 to removing it altogether.
> 
> +1  We are terrible at removing old GUCs and having it around means
> everyone has to decide if they need to change it, so having it is not a
> zero cost.

Are you voting to remove the GUC?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, May 24, 2018 at 02:23:17PM -0400, Alvaro Herrera wrote:
> On 2018-May-24, Bruce Momjian wrote:
> 
> > On Wed, May  2, 2018 at 07:49:42PM -0700, David G. Johnston wrote:
> 
> > > I toss my +1 to removing it altogether.
> > 
> > +1  We are terrible at removing old GUCs and having it around means
> > everyone has to decide if they need to change it, so having it is not a
> > zero cost.
> 
> Are you voting to remove the GUC?

Yes.  Sorry but I am very late on this and maybe too late to vote.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Pushed.  I made a couple of minor changes, in particular I added the
word "one" to this sentence, which was already under discussion:

On 2018-May-24, Justin Pryzby wrote:

> On Thu, May 24, 2018 at 11:30:40AM +0900, Amit Langote wrote:

> > +    possible to show the difference between a plan whose partitions have been
> > +    pruned and one whose partitions haven't.  A typical unoptimized plan for
> > +    this type of table setup is:
> > 
> > "a plan whose partitions have been pruned" sounds a bit off; maybe, "a
> > plan in which partitions have been pruned".
> 
> I wrote:
> 
> "[...] a plan for which partitions have been pruned and for which they have
> not."

"it's possible to show the difference between a plan for which partitions have
been pruned and *one* for which they have not."


Thanks!

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2018-May-23, Justin Pryzby wrote:

> There's two other, wider changes to consider:
> 
>  - should "5.10.4. Partition Pruning" be moved after "5.10.2. Declarative
>    Partitioning", rather than after "5.10.3. Implementation Using Inheritance" ?

I considered that when reorganizing this section, but it seemed more
sensible to me to keep both pruning techniques together rather than put
each one immediately below its partitioning technique.  Maybe I'm wrong
in that.

>  - should we find a unified term for "inheritence-based partitioning" and avoid
>    using the word "partitioning" in that context?  For example: "Partitioning
>    can be implemented using table inheritance[...]".  One possible phrase
>    currently begin used is: "legacy inheritance method".

Yeah, maybe it'd be a good time to do that.  In particular I wondered
whether the section title "Partitioning and Constraint Exclusion" should
be changed somehow to note the fact that it's mostly for the legacy
method.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Fri, Jun 01, 2018 at 03:00:10PM -0400, Alvaro Herrera wrote:
> On 2018-May-23, Justin Pryzby wrote:
> 
> > There's two other, wider changes to consider:
...
> 
> >  - should we find a unified term for "inheritence-based partitioning" and avoid
> >    using the word "partitioning" in that context?  For example: "Partitioning
> >    can be implemented using table inheritance[...]".  One possible phrase
> >    currently begin used is: "legacy inheritance method".
> 
> Yeah, maybe it'd be a good time to do that.  In particular I wondered
> whether the section title "Partitioning and Constraint Exclusion" should
> be changed somehow to note the fact that it's mostly for the legacy
> method.

I made changes to avoid "partition" (which I think should mean a child of
relkind='p', and itself of relkind='r') and "partitioned" (meaning relkind='p'
itself) but left alone most instances of "partitioning".

There's two issues.  One is the unfortunately-named, recommended setting of
constraint_exclusion='partition' :(

And one is this, which I think should be disambiguated from native
list/range/hash partition bounds:

      Use simple equality conditions for list partitioning, or simple range
      tests for range partitioning, as illustrated in the preceding examples.

I'm short on words so maybe someone else can recommend language.

On Fri, Jun 01, 2018 at 02:57:22PM -0400, Alvaro Herrera wrote:
> Pushed.  I made a couple of minor changes, in particular I added the
It looks like you also fixed a double negative - thanks.

Justin

On 01.06.18 23:33, Justin Pryzby wrote:
>>>  - should we find a unified term for "inheritence-based partitioning" and avoid
>>>    using the word "partitioning" in that context?  For example: "Partitioning
>>>    can be implemented using table inheritance[...]".  One possible phrase
>>>    currently begin used is: "legacy inheritance method".
>>
>> Yeah, maybe it'd be a good time to do that.  In particular I wondered
>> whether the section title "Partitioning and Constraint Exclusion" should
>> be changed somehow to note the fact that it's mostly for the legacy
>> method.
> 
> I made changes to avoid "partition" (which I think should mean a child of
> relkind='p', and itself of relkind='r') and "partitioned" (meaning relkind='p'
> itself) but left alone most instances of "partitioning".

Committed.

> There's two issues.  One is the unfortunately-named, recommended setting of
> constraint_exclusion='partition' :(
> 
> And one is this, which I think should be disambiguated from native
> list/range/hash partition bounds:
> 
>       Use simple equality conditions for list partitioning, or simple range
>       tests for range partitioning, as illustrated in the preceding examples.
> 
> I'm short on words so maybe someone else can recommend language.

I'm not worried about those.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2018-Jul-05, Peter Eisentraut wrote:

> On 01.06.18 23:33, Justin Pryzby wrote:
> >>>  - should we find a unified term for "inheritence-based partitioning" and avoid
> >>>    using the word "partitioning" in that context?  For example: "Partitioning
> >>>    can be implemented using table inheritance[...]".  One possible phrase
> >>>    currently begin used is: "legacy inheritance method".
> >>
> >> Yeah, maybe it'd be a good time to do that.  In particular I wondered
> >> whether the section title "Partitioning and Constraint Exclusion" should
> >> be changed somehow to note the fact that it's mostly for the legacy
> >> method.
> > 
> > I made changes to avoid "partition" (which I think should mean a child of
> > relkind='p', and itself of relkind='r') and "partitioned" (meaning relkind='p'
> > itself) but left alone most instances of "partitioning".
> 
> Committed.

Thanks for handling this.

Should we do this in REL_11_STABLE too?  I vote yes.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 6 July 2018 at 09:41, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2018-Jul-05, Peter Eisentraut wrote:
>> Committed.
>
> Thanks for handling this.
>
> Should we do this in REL_11_STABLE too?  I vote yes.

Sorry for now paying much attention to this, but I've read through
what's been committed and I also think PG11 deserves this too.

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

On 2018/07/06 6:55, David Rowley wrote:
> On 6 July 2018 at 09:41, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>> On 2018-Jul-05, Peter Eisentraut wrote:
>>> Committed.
>>
>> Thanks for handling this.
>>
>> Should we do this in REL_11_STABLE too?  I vote yes.
> 
> Sorry for now paying much attention to this, but I've read through
> what's been committed and I also think PG11 deserves this too.

+1

Thanks Justin and Peter.

Regards,
Amit

On 2018-Jul-06, Amit Langote wrote:

> On 2018/07/06 6:55, David Rowley wrote:
> > On 6 July 2018 at 09:41, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> >> On 2018-Jul-05, Peter Eisentraut wrote:
> >>> Committed.
> >>
> >> Thanks for handling this.
> >>
> >> Should we do this in REL_11_STABLE too?  I vote yes.
> > 
> > Sorry for now paying much attention to this, but I've read through
> > what's been committed and I also think PG11 deserves this too.
> 
> +1

Done, thanks :-)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Fri, 11 May 2018 at 17:37, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> 5. The last sentence in caveats, that is,
>
> "Partitioning using these techniques will work well with up to perhaps a
> hundred partitions; don't try to use many thousands of partitions."
>
> should perhaps be reworded as:
>
> "So the legacy inheritance based partitioning will work well with up to
> perhaps a hundred partitions; don't try to use many thousands of partitions."

(digging up 10-month-old thread [1])

There was a report [2] on -general today where someone had a 4000
partition partitioned table and were complaining about memory
consumption in the planner during DELETE.  They didn't mention the
exact version they were using, but mentioned that the problem exists
on 10, 11 and master.  Of course, we're well aware of this issue with
DELETE and UPDATE of large partition hierarchies, Amit has been
working hard with trying to solve it for PG12.

In the -general post, I was just about to point them at the part in
the documents that warn against these large partition hierarchies, but
it looks like the warning was removed in bebc46931a1, or at least
modified to say that constraint exclusion with heritance tables is
slow. I really wonder if we shouldn't put something back in there to
warn against this sort of thing.  It might be a bit late for the
people who've read the docs and done it already, but a warning might
at least stop new people making the mistake.

Hopefully one day we can remove the warning again, but it won't be for PG12.

Thoughts?

[1] https://www.postgresql.org/message-id/6bc4e96a-0e30-e9b6-dcc7-791c7486a491%40lab.ntt.co.jp
[2] https://www.postgresql.org/message-id/739b7a5e-1192-1011-5aa2-41adad55682d%40perfexpert.ch

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

On Sun, Mar 10, 2019 at 10:53:02PM +1300, David Rowley wrote:
> On Fri, 11 May 2018 at 17:37, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> > 5. The last sentence in caveats, that is,
> >
> > "Partitioning using these techniques will work well with up to perhaps a
> > hundred partitions; don't try to use many thousands of partitions."
> >
> > should perhaps be reworded as:
> >
> > "So the legacy inheritance based partitioning will work well with up to
> > perhaps a hundred partitions; don't try to use many thousands of partitions."

> In the -general post, I was just about to point them at the part in
> the documents that warn against these large partition hierarchies, but
> it looks like the warning was removed in bebc46931a1, or at least
> modified to say that constraint exclusion with heritance tables is
> slow. I really wonder if we shouldn't put something back in there to
> warn against this sort of thing.

+1

I believe I was of the same mind when I wrote:
https://www.postgresql.org/message-id/flat/20180525215002.GD14378%40telsasoft.com#c9de33b17fe63cecad4ac30fb1662531

Justin

PS. Sorry to dredge up another 10 month old thread..

On 2019/03/11 0:25, Justin Pryzby wrote:
> On Sun, Mar 10, 2019 at 10:53:02PM +1300, David Rowley wrote:
>> On Fri, 11 May 2018 at 17:37, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> 5. The last sentence in caveats, that is,
>>>
>>> "Partitioning using these techniques will work well with up to perhaps a
>>> hundred partitions; don't try to use many thousands of partitions."
>>>
>>> should perhaps be reworded as:
>>>
>>> "So the legacy inheritance based partitioning will work well with up to
>>> perhaps a hundred partitions; don't try to use many thousands of partitions."
> 
>> In the -general post, I was just about to point them at the part in
>> the documents that warn against these large partition hierarchies, but
>> it looks like the warning was removed in bebc46931a1, or at least
>> modified to say that constraint exclusion with heritance tables is
>> slow. I really wonder if we shouldn't put something back in there to
>> warn against this sort of thing.
> 
> +1
> 
> I believe I was of the same mind when I wrote:
> https://www.postgresql.org/message-id/flat/20180525215002.GD14378%40telsasoft.com#c9de33b17fe63cecad4ac30fb1662531

I agree PG 11 didn't improve things enough to have removed such a warning
from the documentation even for partitioning.  Actually, we only ever had
a warning about constraint exclusion getting slower as more children are
added, but nothing about UPDATE/DELETE planning being slow in itself;
perhaps more importantly, much slower than SELECT.  It seems very hard to
put that in the documentation though.

In PG 10:

Excluding unnecessary partitions is slow, especially as the number of
partitions increases, because constraint exclusion needs to look at each
partition to determine whether it could be excluded.  Also, planning for
UPDATE and DELETE queries is significantly slower than for SELECT queries
for $REASONS.  Given that, it is wise to use up to a few hundred
partitions but not more.


PG 11 moved the needle a bit for SELECT queries:

Excluding unnecessary partitions is slow for UPDATE and DELETE queries,
especially as the number of partitions increases, because constraint
exclusion needs to look at each partition to determine whether it could be
excluded.  Also, planning for UPDATE and DELETE queries is significantly
slower than for SELECT queries for $REASONS.  Given that, it is wise to
use up to a few hundred partitions but not more.

Thanks,
Amit

On Mon, 11 Mar 2019 at 14:33, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> PG 11 moved the needle a bit for SELECT queries:
>
> Excluding unnecessary partitions is slow for UPDATE and DELETE queries,

With those words I expect the user might be surprised that it's still
slow after doing SET enable_partition_pruning = off;

I'm not really talking about constraint exclusion or partition
pruning. The memory growth problem the user was experiencing was down
to the fact that we plan once per partition and each of the
PlannerInfos used for each planner run has a RangeTblEntry for all
partitions. This means if you add one more partition and you get N
partitions more RangeTblEntry items in memory. This is the quadratic
memory growth that I mentioned in the -general post.

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

On 2019/03/11 11:00, David Rowley wrote:
> On Mon, 11 Mar 2019 at 14:33, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> PG 11 moved the needle a bit for SELECT queries:
>>
>> Excluding unnecessary partitions is slow for UPDATE and DELETE queries,
> 
> With those words I expect the user might be surprised that it's still
> slow after doing SET enable_partition_pruning = off;
> 
> I'm not really talking about constraint exclusion or partition
> pruning. The memory growth problem the user was experiencing was down
> to the fact that we plan once per partition and each of the
> PlannerInfos used for each planner run has a RangeTblEntry for all
> partitions. This means if you add one more partition and you get N
> partitions more RangeTblEntry items in memory. This is the quadratic
> memory growth that I mentioned in the -general post.

Yeah, I get it.  As I said in my email, all we have ever mentioned in the
documentation as the reason for queries on partitioned tables being slow
is that partition exclusion is slow and nothing else.  Can we put
quadratic memory growth during planning as the reason for performance
degradation into the documentation?  Maybe we could, but every time I
tried it, it didn't read like user-facing documentation to me.  Do you
have something in mind that we could add?

Thanks,
Amit

On Mon, 11 Mar 2019 at 15:00, David Rowley <david.rowley@2ndquadrant.com> wrote:
>
> On Mon, 11 Mar 2019 at 14:33, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> > PG 11 moved the needle a bit for SELECT queries:
> >
> > Excluding unnecessary partitions is slow for UPDATE and DELETE queries,
>
> With those words I expect the user might be surprised that it's still
> slow after doing SET enable_partition_pruning = off;

I had in mind in 10, 11 and master add a note to mention:

Currently, it is not recommended to have partition hierarchies more
than a few hundred partitions.  Larger partition hierarchies can
suffer from slow planning times with <command>SELECT</command>
queries.  Planning times for <command>UPDATE</command> and
<command>DELETE</command> commands may also suffer slow planning
times, but in addition, memory consumption may also become an issue
due to how the planner currently plans the query once per partition.
These limitations are likely to be resolved in a future version of
<productname>PostgreSQL</productname>.

I've not really thought too much on the fact that the issue also
exists with inheritance tables in earlier version too.

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

On 2019/03/11 11:13, David Rowley wrote:
> On Mon, 11 Mar 2019 at 15:00, David Rowley <david.rowley@2ndquadrant.com> wrote:
>>
>> On Mon, 11 Mar 2019 at 14:33, Amit Langote
>> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>> PG 11 moved the needle a bit for SELECT queries:
>>>
>>> Excluding unnecessary partitions is slow for UPDATE and DELETE queries,
>>
>> With those words I expect the user might be surprised that it's still
>> slow after doing SET enable_partition_pruning = off;
> 
> I had in mind in 10, 11 and master add a note to mention:

Thanks for putting this together.

> Currently, it is not recommended to have partition hierarchies more
> than a few hundred partitions.  Larger partition hierarchies can
> suffer from slow planning times with <command>SELECT</command>
> queries.  Planning times for <command>UPDATE</command> and
> <command>DELETE</command> commands may also suffer slow planning
> times, but in addition, memory consumption may also become an issue
> due to how the planner currently plans the query once per partition.
> These limitations are likely to be resolved in a future version of
> <productname>PostgreSQL</productname>.

How about slightly rewriting the sentence toward the end as:

memory consumption may also become an issue, because planner currently
plans the query once for every partition.

> I've not really thought too much on the fact that the issue also
> exists with inheritance tables in earlier version too.

That's fine maybe.

Thanks,
Amit

On Mon, Mar 11, 2019 at 01:06:08PM +0900, Amit Langote wrote:
> On 2019/03/11 11:13, David Rowley wrote:
> > On Mon, 11 Mar 2019 at 15:00, David Rowley <david.rowley@2ndquadrant.com> wrote:
> >> On Mon, 11 Mar 2019 at 14:33, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> >>> PG 11 moved the needle a bit for SELECT queries:
> >>>
> >>> Excluding unnecessary partitions is slow for UPDATE and DELETE queries,
> >>
> >> With those words I expect the user might be surprised that it's still
> >> slow after doing SET enable_partition_pruning = off;
> > 
> > I had in mind in 10, 11 and master add a note to mention:
> 
> Thanks for putting this together.
> 
> > Currently, it is not recommended to have partition hierarchies more
> > than a few hundred partitions.  Larger partition hierarchies can
> > suffer from slow planning times with <command>SELECT</command>
> > queries.  Planning times for <command>UPDATE</command> and
> > <command>DELETE</command> commands may also suffer slow planning
> > times, but in addition, memory consumption may also become an issue
> > due to how the planner currently plans the query once per partition.
> > These limitations are likely to be resolved in a future version of
> > <productname>PostgreSQL</productname>.

Can I offer the following variation:

| Currently, it is not recommended to have partition hierarchies with more than
| a few hundred partitions.  Larger partition hierarchies may incur long
| planning time.
| In addition, <command>UPDATE</command> and <command>DELETE</command>
| commands on larger hierarchies may cause excessive memory consumption.
| These deficiencies are likely to be fixed in a future release of
| <productname>PostgreSQL</productname>.

On 2019/03/11 13:22, Justin Pryzby wrote:
> On Mon, Mar 11, 2019 at 01:06:08PM +0900, Amit Langote wrote:
>> On 2019/03/11 11:13, David Rowley wrote:
>>> On Mon, 11 Mar 2019 at 15:00, David Rowley <david.rowley@2ndquadrant.com> wrote:
>>>> On Mon, 11 Mar 2019 at 14:33, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>>>>> PG 11 moved the needle a bit for SELECT queries:
>>>>>
>>>>> Excluding unnecessary partitions is slow for UPDATE and DELETE queries,
>>>>
>>>> With those words I expect the user might be surprised that it's still
>>>> slow after doing SET enable_partition_pruning = off;
>>>
>>> I had in mind in 10, 11 and master add a note to mention:
>>
>> Thanks for putting this together.
>>
>>> Currently, it is not recommended to have partition hierarchies more
>>> than a few hundred partitions.  Larger partition hierarchies can
>>> suffer from slow planning times with <command>SELECT</command>
>>> queries.  Planning times for <command>UPDATE</command> and
>>> <command>DELETE</command> commands may also suffer slow planning
>>> times, but in addition, memory consumption may also become an issue
>>> due to how the planner currently plans the query once per partition.
>>> These limitations are likely to be resolved in a future version of
>>> <productname>PostgreSQL</productname>.
> 
> Can I offer the following variation:
> 
> | Currently, it is not recommended to have partition hierarchies with more than
> | a few hundred partitions.  Larger partition hierarchies may incur long
> | planning time.
> | In addition, <command>UPDATE</command> and <command>DELETE</command>
> | commands on larger hierarchies may cause excessive memory consumption.
> | These deficiencies are likely to be fixed in a future release of
> | <productname>PostgreSQL</productname>.

Says essentially the same thing but with fewer words, so +1.

Now the question is where to put this text?  Currently, we have:

5.10. Table Partitioning
  5.10.1. Overview
  5.10.2. Declarative Partitioning
  5.10.3. Implementation Using Inheritance
  5.10.4. Partition Pruning
  5.10.5. Partitioning and Constraint Exclusion

Should we add 5.10.6 Notes for the above "note", or should it be stuffed
under one of the existing sub-headings?

Thanks,
Amit

On Mon, Mar 11, 2019 at 12:30 AM Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Now the question is where to put this text?  Currently, we have:
>
> 5.10. Table Partitioning
>   5.10.1. Overview
>   5.10.2. Declarative Partitioning
>   5.10.3. Implementation Using Inheritance
>   5.10.4. Partition Pruning
>   5.10.5. Partitioning and Constraint Exclusion
>
> Should we add 5.10.6 Notes for the above "note", or should it be stuffed
> under one of the existing sub-headings?

I think it should be added to one of the existing sub-headings.  I
suggest adding it to the end of 5.10.1 and rephrasing it so that it
makes clearer the distinction between what will happen with
inheritance and what will happen with table partitioning, e.g.

When using either declarative partitioning or table inheritance,
partitioning hierarchies with more than a few hundred partitions are
not currently recommended. Larger partition hierarchies may incur long
planning time, and especially in the case of UPDATE and DELETE,
excessive memory usage.  When inheritance is used, see also the
limitations described in Section 5.10.5, Partitioning and Constraint
Exclusion.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Wed, 13 Mar 2019 at 04:07, Robert Haas <robertmhaas@gmail.com> wrote:
> I think it should be added to one of the existing sub-headings.  I
> suggest adding it to the end of 5.10.1 and rephrasing it so that it
> makes clearer the distinction between what will happen with
> inheritance and what will happen with table partitioning, e.g.
>
> When using either declarative partitioning or table inheritance,
> partitioning hierarchies with more than a few hundred partitions are
> not currently recommended. Larger partition hierarchies may incur long
> planning time, and especially in the case of UPDATE and DELETE,
> excessive memory usage.  When inheritance is used, see also the
> limitations described in Section 5.10.5, Partitioning and Constraint
> Exclusion.

I think I've done that in the attached patch.  However, do think the
just saying "excessive memory usage" seems strange without prefixing
it with "can result in" and dropping the "especially".  I'm fairly
used to having my wording debated, so I've left your words in the
patch.

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

On 2019/03/13 8:28, David Rowley wrote:
> On Wed, 13 Mar 2019 at 04:07, Robert Haas <robertmhaas@gmail.com> wrote:
>> I think it should be added to one of the existing sub-headings.  I
>> suggest adding it to the end of 5.10.1 and rephrasing it so that it
>> makes clearer the distinction between what will happen with
>> inheritance and what will happen with table partitioning, e.g.

+1.

>> When using either declarative partitioning or table inheritance,
>> partitioning hierarchies with more than a few hundred partitions are
>> not currently recommended. Larger partition hierarchies may incur long
>> planning time, and especially in the case of UPDATE and DELETE,
>> excessive memory usage.  When inheritance is used, see also the
>> limitations described in Section 5.10.5, Partitioning and Constraint
>> Exclusion.
> 
> I think I've done that in the attached patch.  However, do think the
> just saying "excessive memory usage" seems strange without prefixing
> it with "can result in" and dropping the "especially".
FWIW, I've gotten used to reading the kind of English that Robert wrote
(meaning it makes perfect sense to me), but wording tweaks you suggest
will work to.

Thanks,
Amit

On Tue, Mar 12, 2019 at 7:28 PM David Rowley
<david.rowley@2ndquadrant.com> wrote:
> I think I've done that in the attached patch.

Cool, thanks.

> However, do think the
> just saying "excessive memory usage" seems strange without prefixing
> it with "can result in" and dropping the "especially".  I'm fairly
> used to having my wording debated, so I've left your words in the
> patch.

I'm not direly opposed to that.  I included "especially" so as not to
rule out the possibility that there might be cases other than UPDATE
and DELETE that, in some circumstances, also use a lot of memory.  I
didn't prefix it with "can result in" because I don't think English
grammar requires it to be there.  It would be grammatically correct to
say "Larger partitioning hierarchies may incur long planning time and
excessive memory usage," and I don't think that injecting an
appositive phrase before "excessive memory usage" changes that
calculus.  However, somebody might find your way easier to follow.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, 14 Mar 2019 at 02:10, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Tue, Mar 12, 2019 at 7:28 PM David Rowley
> <david.rowley@2ndquadrant.com> wrote:
> > I think I've done that in the attached patch.
>
> Cool, thanks.

Just so I don't forget about this, I've added it to the July 'fest.

https://commitfest.postgresql.org/23/2065/

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

On Thu, 21 Mar 2019 at 00:51, David Rowley <david.rowley@2ndquadrant.com> wrote:
> Just so I don't forget about this, I've added it to the July 'fest.
>
> https://commitfest.postgresql.org/23/2065/

Now that we have 428b260f8, I think the version of this that goes into
master should be more like the attached.

I think the original patch is fine for the back branches.

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

On 2019/04/11 12:34, David Rowley wrote:
> On Thu, 21 Mar 2019 at 00:51, David Rowley <david.rowley@2ndquadrant.com> wrote:
>> Just so I don't forget about this, I've added it to the July 'fest.
>>
>> https://commitfest.postgresql.org/23/2065/
> 
> Now that we have 428b260f8, I think the version of this that goes into
> master should be more like the attached.

Thanks, looks good.

I've posted a patch last week on the "speed up partition planning" thread
[1] which modifies ddl.sgml to remove the text about UPDATE/DELETE using
constraint exclusion under the covers.  Do you think there's any merit to
combining that with this one?

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/4f049572-9440-3c99-afa1-f7ca7f38fe80%40lab.ntt.co.jp

On Thu, 11 Apr 2019 at 16:06, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
>
> On 2019/04/11 12:34, David Rowley wrote:
> > Now that we have 428b260f8, I think the version of this that goes into
> > master should be more like the attached.
>
> Thanks, looks good.

Thanks for looking.

> I've posted a patch last week on the "speed up partition planning" thread
> [1] which modifies ddl.sgml to remove the text about UPDATE/DELETE using
> constraint exclusion under the covers.  Do you think there's any merit to
> combining that with this one?

Probably separate is better. I don't think anything you're proposing
there is for back-patching, but I think the original patch over here
should be.

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

On 2019/04/11 13:50, David Rowley wrote:
> On Thu, 11 Apr 2019 at 16:06, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> I've posted a patch last week on the "speed up partition planning" thread
>> [1] which modifies ddl.sgml to remove the text about UPDATE/DELETE using
>> constraint exclusion under the covers.  Do you think there's any merit to
>> combining that with this one?
> 
> Probably separate is better. I don't think anything you're proposing
> there is for back-patching, but I think the original patch over here
> should be.

OK, no problem.  I just thought to point out my patch because you've
posted a version of the patch here for HEAD *because of* 428b260f8, the
commit which also obsoleted the text that the other patch fixes.

Anyway, let's leave the other patch on its own thread where there are a
few other things to be sorted out.

Thanks,
Amit

On Thu, Apr 11, 2019 at 03:34:30PM +1200, David Rowley wrote:
> On Thu, 21 Mar 2019 at 00:51, David Rowley <david.rowley@2ndquadrant.com> wrote:
> > Just so I don't forget about this, I've added it to the July 'fest.
> >
> > https://commitfest.postgresql.org/23/2065/
> 
> Now that we have 428b260f8, I think the version of this that goes into
> master should be more like the attached.

I tweaked this patch some more (sorry):
 - remove "currently" since that's not expected to be changed (right?);
 - remove "especially";
 - refer to "partition hierarchies" not "partitioning hierarchies";
 - rewrite bit about "When partition pruning is not possible"

Also, I noticed awhile ago while grepping for "probably be fixed in future
releases" that some items under ddl-inherit-caveats are actually possible for
relkind=p partitions in v11.  I assume those will never be implemented for
inheritence partitioning, so I propose another update to docs (if preferred,
I'll bring up on a new thread).

 - unique constraints on parent table;
 - FK constraints on parent table;

Note that FK constraints *referencing* a partitiond table are possible in v12
but not in v11.  So if there's any finer-grained update to documentation of the
individual limitations, it'd need to be tweaked for back branches (v10 and 11).

Justin

On Thu, 11 Apr 2019 at 17:40, Justin Pryzby <pryzby@telsasoft.com> wrote:
> I tweaked this patch some more (sorry):
>  - remove "currently" since that's not expected to be changed (right?);

Seems like a good idea.  I think the way we exclude inheritance child
relations will never scale well. Other improvements that we'll see
will most likely be as a consequence of speeding up declarative
partitioning. For example the planner improvements in PG12 just did
that for UPDATE/DELETE.

>  - remove "especially";

I think that likely needs to be kept for the PG11 version. I was
hoping it was stop a casual tester testing a SELECT and seeing that
it's not so bad only to find later that UPDATE/DELETE OOMs.

>  - refer to "partition hierarchies" not "partitioning hierarchies";

fine

>  - rewrite bit about "When partition pruning is not possible"

fine.

> Also, I noticed awhile ago while grepping for "probably be fixed in future
> releases" that some items under ddl-inherit-caveats are actually possible for
> relkind=p partitions in v11.  I assume those will never be implemented for
> inheritence partitioning, so I propose another update to docs (if preferred,
> I'll bring up on a new thread).

Not sure about that. It may be very simple to implement if we one day
get global indexes. It may just be a matter of pointing all the tables
at the same index and letting the wonders of global indexes handle all
the hard stuff. I'm not that excited about removing that. I'd be
equally excited about adding the text if it wasn't already there and
you were proposing to add it.

>  - unique constraints on parent table;
>  - FK constraints on parent table;
>
> Note that FK constraints *referencing* a partitiond table are possible in v12
> but not in v11.  So if there's any finer-grained update to documentation of the
> individual limitations, it'd need to be tweaked for back branches (v10 and 11).

Don't we just need to remove or update:

     <listitem>
      <para>
       While primary keys are supported on partitioned tables, foreign
       keys referencing partitioned tables are not supported.  (Foreign key
       references from a partitioned table to some other table are supported.)
      </para>
     </listitem>

I didn't follow this work, but on testing, I see the foreign key does
not CASCADE when doing DETACH PARTITION, it errors instead. Perhaps
that's worth a mention here.

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

On Fri, Apr 12, 2019 at 02:01:39PM +1200, David Rowley wrote:
> On Thu, 11 Apr 2019 at 17:40, Justin Pryzby <pryzby@telsasoft.com> wrote:
> > I tweaked this patch some more (sorry):
> >  - remove "especially";
> 
> I think that likely needs to be kept for the PG11 version. I was
> hoping it was stop a casual tester testing a SELECT and seeing that
> it's not so bad only to find later that UPDATE/DELETE OOMs.

With "especially", it reads as if "excessive memory usage" might happen for
SELECT, and it'll be additionally worse for UPDATE/DELETE.

Without "especially", it makes "excessive RAM use" apply only to UPDATE/DELETE,
which I think is what's intended.

|Larger partition hierarchies may incur long planning time, and [especially] in
|the case of <command>UPDATE</command> and <command>DELETE</command>, excessive
|memory usage.

I think as long as UPDATE/DELETE are specifically mentioned, that would handle
your concern.  If I were to suggest an alternative:

|Larger partition hierarchies may incur long planning time; and, in
|the case of <command>UPDATE</command> and <command>DELETE</command>, may also
|incur excessive memory usage.

..after which I'll stop wrestling with words.

Justin