Hi,
On Thu, Nov 22, 2018 at 11:27 AM David Rowley
<david.rowley@2ndquadrant.com> wrote:
>
> On Mon, 5 Nov 2018 at 10:46, David Rowley <david.rowley@2ndquadrant.com> wrote:
> > On 1 November 2018 at 22:05, Antonin Houska <ah@cybertec.at> wrote:
> > > I think these conditions are too restrictive:
> > >
> > > /*
> > > * Determine if these pathkeys match the partition order, or reverse
> > > * partition order. It can't match both, so only go to the trouble of
> > > * checking the reverse order when it's not in ascending partition
> > > * order.
> > > */
> > > partition_order = pathkeys_contained_in(pathkeys,
> > > partition_pathkeys);
> > > partition_order_desc = !partition_order &&
> > > pathkeys_contained_in(pathkeys,
> > > partition_pathkeys_desc);
> > >
>
> > The problem with doing that is that if the partition keys are better
> > than the pathkeys then we'll most likely fail to generate any
> > partition keys at all due to lack of any existing eclass to use for
> > the pathkeys. It's unsafe to use just the prefix in this case as the
> > eclass may not have been found due to, for example one of the
> > partition keys having a different collation than the required sort
> > order of the query. In other words, we can't rely on a failure to
> > create the pathkey meaning that a more strict sort order is not
> > required.
>
> I had another look at this patch and it seems okay just to add a new
> flag to build_partition_pathkeys() to indicate if the pathkey List was
> truncated or not. In generate_mergeappend_paths() we can then just
> check that flag before checking if the partiiton pathkeys are
> contained in pathkeys. It's fine if the partition keys were truncated
> for the reverse of that check.
>
> I've done this in the attached and added additional regression tests
> for this case.
I started to look at v5.
If I understand correctly, the new behavior is controlled by
partitions_are_ordered(), but it only checks for declared partitions,
not partitions that survived pruning. Did I miss something or is it
the intended behavior? Also, generate_mergeappend_paths should
probably be renamed to something like generate_sortedappend_paths
since it can now generate either Append or MergeAppend paths.
I'm also wondering if that's ok to only generate either a (sorted)
Append or a MergeAppend. Is it possible that in some cases it's
better to have a MergeAppend rather than a sorted Append, given that
MergeAppend is parallel-aware and the sorted Append isn't?