pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Change "long" numGroups fields to be Cardinality (i.e., double).
Date: Sun, 02 Nov 2025 21:57:59 +0000
Message-ID: <E1vFg5H-004k5e-2o@gemulon.postgresql.org> (raw)

Change "long" numGroups fields to be Cardinality (i.e., double).

We've been nibbling away at removing uses of "long" for a long time,
since its width is platform-dependent.  Here's one more: change the
remaining "long" fields in Plan nodes to Cardinality, since the three
surviving examples all represent group-count estimates.  The upstream
planner code was converted to Cardinality some time ago; for example
the corresponding fields in Path nodes are type Cardinality, as are
the arguments of the make_foo_path functions.  Downstream in the
executor, it turns out that these all feed to the table-size argument
of BuildTupleHashTable.  Change that to "double" as well, and fix it
so that it safely clamps out-of-range values to the uint32 limit of
simplehash.h, as was not being done before.

Essentially, this is removing all the artificial datatype-dependent
limitations on these values from upstream processing, and applying
just one clamp at the moment where we're forced to do so by the
datatype choices of simplehash.h.

Also, remove BuildTupleHashTable's misguided attempt to enforce
work_mem/hash_mem_limit.  It doesn't have enough information
(particularly not the expected tuple width) to do that accurately,
and it has no real business second-guessing the caller's choice.
For all these plan types, it's really the planner's responsibility
to not choose a hashed implementation if the hashtable is expected
to exceed hash_mem_limit.  The previous patch improved the
accuracy of those estimates, and even if BuildTupleHashTable had
more information it should arrive at the same conclusions.

Reported-by: Jeff Janes <jeff.janes@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/CAMkU=1zia0JfW_QR8L5xA2vpa0oqVuiapm78h=WpNsHH13_9uw@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/8f29467c57f44cc2cdd9e4e53c6ab1b07375d5b4

Modified Files
--------------
src/backend/executor/execGrouping.c       | 36 ++++++++++++++++---------------
src/backend/executor/nodeAgg.c            | 30 ++++++++++++++------------
src/backend/executor/nodeRecursiveunion.c |  1 -
src/backend/executor/nodeSetOp.c          |  1 -
src/backend/executor/nodeSubplan.c        | 19 +++++++---------
src/backend/optimizer/path/costsize.c     | 26 ----------------------
src/backend/optimizer/plan/createplan.c   | 26 ++++++----------------
src/include/executor/executor.h           |  2 +-
src/include/nodes/plannodes.h             |  6 +++---
src/include/optimizer/optimizer.h         |  1 -
src/include/optimizer/planmain.h          |  2 +-
11 files changed, 55 insertions(+), 95 deletions(-)



Message-ID: <E1vFg5H-004k5e-2o@gemulon.postgresql.org>
Permalink:  ../E1vFg5H-004k5e-2o@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1vFg5H-004k5e-2o@gemulon.postgresql.org

 · 

reply

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: tgl@sss.pgh.pa.us, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Change "long" numGroups fields to be Cardinality (i.e., double).
  In-Reply-To: <E1vFg5H-004k5e-2o@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