From: David Rowley <drowley@postgresql.org>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Fix incorrect slot type in BuildTupleHashTableExt
Date: Tue, 17 Dec 2024 23:06:28 +0000
Message-ID: <E1tNge3-0006NP-Mp@gemulon.postgresql.org> (raw)
Fix incorrect slot type in BuildTupleHashTableExt
0f5738202 adjusted the execGrouping.c code so it made use of ExprStates to
generate hash values. That commit made a wrong assumption that the slot
type to pass to ExecBuildHash32FromAttrs() is always &TTSOpsMinimalTuple.
That's not the case as the slot type depends on the slot type passed to
LookupTupleHashEntry(), which for nodeRecursiveunion.c, could be any of
the current slot types.
Here we fix this by adding a new parameter to BuildTupleHashTableExt()
to allow the slot type to be passed in. In the case of nodeSubplan.c
and nodeAgg.c the slot type is always &TTSOpsVirtual, so for both of
those cases, it's beneficial to pass the known slot type as that allows
ExecBuildHash32FromAttrs() to skip adding the tuple deform step to the
resulting ExprState. Another possible fix would have been to have
ExecBuildHash32FromAttrs() set "fetch.kind" to NULL so that
ExecComputeSlotInfo() always determines the EEOP_INNER_FETCHSOME is
required, however, that option isn't favorable as slows down aggregation
and hashed subplan evaluation due to the extra (needless) deform step.
Thanks to Nathan Bossart for bisecting to find the offending commit
based on Paul's report.
Reported-by: Paul Ramsey <pramsey@cleverelephant.ca>
Discussion: https://postgr.es/m/99F064C1-B3EB-4BE7-97D2-D2A0AA487A71@cleverelephant.ca
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/d96d1d5152f30d15678e08e75b42756101b7cab6
Modified Files
--------------
src/backend/executor/execGrouping.c | 5 ++++-
src/backend/executor/nodeAgg.c | 1 +
src/backend/executor/nodeRecursiveunion.c | 2 ++
src/backend/executor/nodeSetOp.c | 1 +
src/backend/executor/nodeSubplan.c | 7 +++++++
src/include/executor/executor.h | 1 +
src/test/regress/expected/with.out | 20 ++++++++++++++++++++
src/test/regress/sql/with.sql | 14 ++++++++++++++
8 files changed, 50 insertions(+), 1 deletion(-)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: drowley@postgresql.org, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Fix incorrect slot type in BuildTupleHashTableExt
In-Reply-To: <E1tNge3-0006NP-Mp@gemulon.postgresql.org>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox