Doubts about pushing LIMIT to MergeAppendPath - Mailing list pgsql-hackers

From Antonin Houska
Subject Doubts about pushing LIMIT to MergeAppendPath
Date
Msg-id 22314.1541072925@localhost
Whole thread Raw
Responses Re: Doubts about pushing LIMIT to MergeAppendPath
List pgsql-hackers
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.

I tried to reproduce the issue by applying the "Incremental sort" [2] patch
and running the following example:

CREATE TABLE t(i int, j int);
CREATE TABLE t1() INHERITS (t);
CREATE INDEX ON t1(i, j);
INSERT INTO t1(i, j) VALUES (1, 0), (1, 1);
CREATE TABLE t2() INHERITS (t);
CREATE INDEX ON t2(i, j);
INSERT INTO t2(i, j) VALUES (1, 0), (1, 1);

ANALYZE;

SELECT * FROM t ORDER BY i, j DESC LIMIT 1;

I expected the MergeAppend plan to apply the limit and thus prevent the
incremental sort node from receiving the first tuple that it should emit,
however the query yielded correct result. I think the reason is that the
MergeAppendPath.limit_tuples field is only used for cost estimates, but not
enforced during execution. Is that intended?

I thought this could be better approach to the limit push-down

    if (root->limit_tuples > 0 && root->parse->sortClause == NIL &&
        bms_equal(rel->relids, root->all_baserels))
        pathnode->limit_tuples = root->limit_tuples;
    else
        pathnode->limit_tuples = -1.0;

however it will stop working as soon as the incremental sort patch (which can
is used below the upper planner) gets applied.

Finally I think we should be able to apply the limit to generic path, not only
to MergeAppendPath. I just don't know how to check when it's correct. Does
anyone have an idea?

[1] https://commitfest.postgresql.org/20/1850/

[2] https://commitfest.postgresql.org/20/1124/

--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26, A-2700 Wiener Neustadt
Web: https://www.cybertec-postgresql.com


pgsql-hackers by date:

Previous
From: Dilip Kumar
Date:
Subject: Re: CVE-2017-7484-induced bugs, or, btree cmp functions are not leakproof?
Next
From: Daniel Gustafsson
Date:
Subject: Re: PG vs macOS Mojave