pgsql: Avoid using lcons and list_delete_first where it's easy to do so

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Avoid using lcons and list_delete_first where it's easy to do so
Date: 2019-07-17 15:15:46
Message-ID: E1hnlek-0004p0-Or@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Avoid using lcons and list_delete_first where it's easy to do so.

Formerly, lcons was about the same speed as lappend, but with the new
List implementation, that's not so; with a long List, data movement
imposes an O(N) cost on lcons and list_delete_first, but not lappend.

Hence, invent list_delete_last with semantics parallel to
list_delete_first (but O(1) cost), and change various places to use
lappend and list_delete_last where this can be done without much
violence to the code logic.

There are quite a few places that construct result lists using lcons not
lappend. Some have semantic rationales for that; I added comments about
it to a couple that didn't have them already. In many such places though,
I think the coding is that way only because back in the dark ages lcons
was faster than lappend. Hence, switch to lappend where this can be done
without causing semantic changes.

In ExecInitExprRec(), this results in aggregates and window functions that
are in the same plan node being executed in a different order than before.
Generally, the executions of such functions ought to be independent of
each other, so this shouldn't result in visibly different query results.
But if you push it, as one regression test case does, you can show that
the order is different. The new order seems saner; it's closer to
the order of the functions in the query text. And we never documented
or promised anything about this, anyway.

Also, in gistfinishsplit(), don't bother building a reverse-order list;
it's easy now to iterate backwards through the original list.

It'd be possible to go further towards removing uses of lcons and
list_delete_first, but it'd require more extensive logic changes,
and I'm not convinced it's worth it. Most of the remaining uses
deal with queues that probably never get long enough to be worth
sweating over. (Actually, I doubt that any of the changes in this
patch will have measurable performance effects either. But better
to have good examples than bad ones in the code base.)

Patch by me, thanks to David Rowley and Daniel Gustafsson for review.

Discussion: https://postgr.es/m/21272.1563318411@sss.pgh.pa.us

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/d97b714a219959a50f9e7b37ded674f5132f93f3

Modified Files
--------------
src/backend/access/gist/gist.c | 21 +++++----------------
src/backend/catalog/heap.c | 4 ++--
src/backend/commands/cluster.c | 2 +-
src/backend/commands/lockcmds.c | 4 ++--
src/backend/commands/tablecmds.c | 5 +++++
src/backend/commands/typecmds.c | 2 +-
src/backend/executor/execExpr.c | 4 ++--
src/backend/nodes/list.c | 24 ++++++++++++++++++++++++
src/backend/optimizer/util/clauses.c | 15 +++++++--------
src/backend/optimizer/util/plancat.c | 13 ++++++++++---
src/backend/parser/parse_agg.c | 2 +-
src/backend/rewrite/rewriteHandler.c | 12 ++++++------
src/backend/utils/adt/selfuncs.c | 6 +++---
src/include/nodes/pg_list.h | 1 +
src/test/regress/expected/aggregates.out | 4 ++--
15 files changed, 72 insertions(+), 47 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2019-07-17 17:05:04 pgsql: Fix sepgsql test results for commit d97b714a2.
Previous Message Thomas Munro 2019-07-17 07:16:00 Re: UCT (Re: pgsql: Update time zone data files to tzdata release 2019a.)