Re: [HACKERS] UPDATE of partition key

On 2017/07/26 6:07, Robert Haas wrote:
> On Thu, Jul 13, 2017 at 1:09 PM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
>> Attached is a WIP patch (make_resultrels_ordered.patch) that generates
>> the result rels in canonical order. This patch is kept separate from
>> the update-partition-key patch, and can be applied on master branch.
>
> I suspect this isn't correct for a table that contains wCTEs, because
> there would in that case be multiple result relations.
> 
> I think we should always expand in bound order rather than only when
> it's a result relation. I think for partition-wise join, we're going
> to want to do it this way for all relations in the query, or at least
> for all relations in the query that might possibly be able to
> participate in a partition-wise join.  If there are multiple cases
> that are going to need this ordering, it's hard for me to accept the
> idea that it's worth the complexity of trying to keep track of when we
> expanded things in one order vs. another.  There are other
> applications of having things in bound order too, like MergeAppend ->
> Append strength-reduction (which might not be legal anyway if there
> are list partitions with multiple, non-contiguous list bounds or if
> any NULL partition doesn't end up in the right place in the order, but
> there will be lots of cases where it can work).

Sorry to be responding this late to the Amit's make_resultrel_ordered
patch itself, but I agree that we should teach the planner to *always*
expand partitioned tables in the partition bound order.

When working on something else, I ended up writing a prerequisite patch
that refactors RelationGetPartitionDispatchInfo() to not be too tied to
its current usage for tuple-routing, so that it can now be used in the
planner (for example, in expand_inherited_rtentry(), instead of
find_all_inheritors()).  If we could adopt that patch, we can focus on the
update partition row movement issues more closely on this thread, rather
than the concerns about the order that planner puts partitions into.

I checked that we get the same result relation order with both the
patches, but I would like to highlight a notable difference here between
the approaches taken by our patches.  In my patch, I have now taught
RelationGetPartitionDispatchInfo() to lock *only* the partitioned tables
in the tree, because we need to look at its partition descriptor to
collect partition OIDs and bounds.  We can defer locking (and opening the
relation descriptor of) leaf partitions to a point where planner has
determined that the partition will be accessed after all (not pruned),
which will be done in a separate patch of course.

Sorry again that I didn't share this patch sooner.

Thanks,
Amit

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers