pgsql: Improve hash_create()'s API for some added robustness.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: pgsql: Improve hash_create()'s API for some added robustness.
Date: 2020-12-15 16:39:14
Message-ID: E1kpDM2-0000c0-QK@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Improve hash_create()'s API for some added robustness.

Invent a new flag bit HASH_STRINGS to specify C-string hashing, which
was formerly the default; and add assertions insisting that exactly
one of the bits HASH_STRINGS, HASH_BLOBS, and HASH_FUNCTION be set.
This is in hopes of preventing recurrences of the type of oversight
fixed in commit a1b8aa1e4 (i.e., mistakenly omitting HASH_BLOBS).

Also, when HASH_STRINGS is specified, insist that the keysize be
more than 8 bytes. This is a heuristic, but it should catch
accidental use of HASH_STRINGS for integer or pointer keys.
(Nearly all existing use-cases set the keysize to NAMEDATALEN or
more, so there's little reason to think this restriction should
be problematic.)

Tweak hash_create() to insist that the HASH_ELEM flag be set, and
remove the defaults it had for keysize and entrysize. Since those
defaults were undocumented and basically useless, no callers
omitted HASH_ELEM anyway.

Also, remove memset's zeroing the HASHCTL parameter struct from
those callers that had one. This has never been really necessary,
and while it wasn't a bad coding convention it was confusing that
some callers did it and some did not. We might as well save a few
cycles by standardizing on "not".

Also improve the documentation for hash_create().

In passing, improve reinit.c's usage of a hash table by storing
the key as a binary Oid rather than a string; and, since that's
a temporary hash table, allocate it in CurrentMemoryContext for
neatness.

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

Branch
------
master

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

Modified Files
--------------
contrib/dblink/dblink.c | 3 +-
contrib/pg_stat_statements/pg_stat_statements.c | 1 -
contrib/postgres_fdw/connection.c | 5 +-
contrib/postgres_fdw/shippable.c | 1 -
contrib/tablefunc/tablefunc.c | 3 +-
src/backend/access/gist/gistbuildbuffers.c | 1 -
src/backend/access/hash/hashpage.c | 1 -
src/backend/access/heap/rewriteheap.c | 2 -
src/backend/access/transam/xlogutils.c | 1 -
src/backend/catalog/pg_enum.c | 1 -
src/backend/catalog/pg_inherits.c | 1 -
src/backend/commands/async.c | 1 -
src/backend/commands/prepare.c | 4 +-
src/backend/commands/sequence.c | 1 -
src/backend/executor/execPartition.c | 1 -
src/backend/nodes/extensible.c | 4 +-
src/backend/optimizer/util/predtest.c | 1 -
src/backend/optimizer/util/relnode.c | 1 -
src/backend/parser/parse_oper.c | 1 -
src/backend/partitioning/partdesc.c | 6 +-
src/backend/postmaster/autovacuum.c | 1 -
src/backend/postmaster/checkpointer.c | 1 -
src/backend/postmaster/pgstat.c | 6 --
src/backend/replication/logical/relation.c | 3 -
src/backend/replication/logical/reorderbuffer.c | 3 -
src/backend/replication/logical/tablesync.c | 1 -
src/backend/replication/pgoutput/pgoutput.c | 4 --
src/backend/storage/buffer/bufmgr.c | 1 -
src/backend/storage/buffer/localbuf.c | 1 -
src/backend/storage/file/reinit.c | 25 +++-----
src/backend/storage/ipc/shmem.c | 10 ++-
src/backend/storage/ipc/standby.c | 1 -
src/backend/storage/lmgr/lock.c | 1 -
src/backend/storage/lmgr/lwlock.c | 1 -
src/backend/storage/lmgr/predicate.c | 4 --
src/backend/storage/smgr/smgr.c | 1 -
src/backend/storage/sync/sync.c | 1 -
src/backend/tsearch/ts_typanalyze.c | 1 -
src/backend/utils/adt/array_typanalyze.c | 2 -
src/backend/utils/adt/jsonfuncs.c | 6 +-
src/backend/utils/adt/pg_locale.c | 1 -
src/backend/utils/adt/ri_triggers.c | 3 -
src/backend/utils/adt/ruleutils.c | 4 +-
src/backend/utils/cache/attoptcache.c | 1 -
src/backend/utils/cache/evtcache.c | 1 -
src/backend/utils/cache/relcache.c | 2 -
src/backend/utils/cache/relfilenodemap.c | 10 ++-
src/backend/utils/cache/spccache.c | 1 -
src/backend/utils/cache/ts_cache.c | 3 -
src/backend/utils/cache/typcache.c | 2 -
src/backend/utils/fmgr/dfmgr.c | 3 +-
src/backend/utils/fmgr/fmgr.c | 1 -
src/backend/utils/hash/dynahash.c | 83 ++++++++++++++++++-------
src/backend/utils/mmgr/portalmem.c | 2 +-
src/backend/utils/time/combocid.c | 1 -
src/include/utils/hsearch.h | 22 ++++---
src/pl/plperl/plperl.c | 5 +-
src/pl/plpgsql/src/pl_comp.c | 1 -
src/pl/plpgsql/src/pl_exec.c | 2 -
src/pl/plpython/plpy_plpymodule.c | 1 -
src/pl/plpython/plpy_procedure.c | 1 -
src/pl/tcl/pltcl.c | 2 -
src/timezone/pgtz.c | 4 +-
63 files changed, 112 insertions(+), 158 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Peter Eisentraut 2020-12-15 21:05:03 pgsql: Clean up ancient test style
Previous Message Andrew Dunstan 2020-12-15 15:53:48 pgsql: Use native methods to open input in TestLib::slurp_file on Windo