Re: Doubts about pushing LIMIT to MergeAppendPath

On 11/01/2018 08:11 PM, Tomas Vondra wrote:
> On 11/01/2018 12:48 PM, Antonin Houska wrote:
>> Review of [1] made me think of this optimization, currently used only in
>> create_merge_append_path():
>>
>> /*
>> * Apply query-wide LIMIT if known and path is for sole base relation.
>> * (Handling this at this low level is a bit klugy.)
>> */
>> if (bms_equal(rel->relids, root->all_baserels))
>> pathnode->limit_tuples = root->limit_tuples;
>> else
>> pathnode->limit_tuples = -1.0;
>>
>> Currently it's not a problem because the output of MergeAppend plan is not
>> likely to be re-sorted, but I don't think data correctness should depend on
>> cost evaluation. Instead, -1 should be set here if there's any chance that the
>> output will be sorted again.
>>
>
> So you're saying we might end up with a plan like this:
>
> Limit
> -> Sort
> -> MergeAppend
> -> SeqScan on t
>
> In which case we'd pass the wrong limit_tuples to the MergeAppend?
>
> Hmmm, that would depend on us building MergeAppend node that does not
> match the expected pathkeys, and pick it instead of plain Append node.
> I'm not sure that's actually possible, but maybe it is ...
>

OK, so the reason is that when building child paths, we don't keep the
pathkeys unless it matches the "interesting" pathkeys.

So for example we may have an IndexPath, but with pathkeys=NIL if the
index does not match the ORDER BY we need. So we don't even build the
MergeAppend paths, as generate_mergeappend_paths iterates over child
pathkeys (and we don't have any).

We might have two child paths, one with pathkeys (matching the ORDER BY)
and one without pathkeys. But at that point the optimization does not
apply, because it checks for "single base relation".

At least that's my understanding of add_paths_to_append_rel().

regards

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