public inbox for [email protected]  
help / color / mirror / Atom feed
From: Robert Haas <[email protected]>
To: John Naylor <[email protected]>
Cc: Lukas Fittl <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: Dian Fay <[email protected]>
Cc: Matheus Alcantara <[email protected]>
Cc: Jakub Wartak <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: pg_plan_advice
Date: Tue, 13 Jan 2026 10:09:14 -0500
Message-ID: <CA+Tgmoa6iQAz-RmAd3tWc=Rgr6beZDetZuA7o298tn=6prLhsA@mail.gmail.com> (raw)
In-Reply-To: <CANWCAZZgKnf8dNOd_w03n88NqOfmMnMv2=D8_Oy6ADGyiMq+cg@mail.gmail.com>
References: <CA+TgmoZ-Jh1T6QyWoCODMVQdhTUPYkaZjWztzP1En4=ZHoKPzw@mail.gmail.com>
	<CAKZiRmxtJAFG7e1+Vs9B8ngON=AOzJbuws+1ZeH4LsbJh5AzoQ@mail.gmail.com>
	<CA+TgmoY9Ne_Sh10u6LSPc3wvOQPLp3kF9nBp3nqJEG2JGF2QiA@mail.gmail.com>
	<CA+Tgmoa57S6mP=aTOXH2-gDAL4TMO1WbGgrHSg0s6J4zUH=04g@mail.gmail.com>
	<[email protected]>
	<CA+Tgmoaf__2B0BUL+vrg28P+3buX=Ti-kybqkHiLTtFrrCfzuA@mail.gmail.com>
	<CA+TgmoYpcLNOuypOTdgCSLW7FuA=t6BtB3meTARHX2-Dj_81xQ@mail.gmail.com>
	<[email protected]>
	<CA+TgmoZjv9OyFu1Gkt78w0vWEti8S33w8trYHmErf-GMmGSi=w@mail.gmail.com>
	<[email protected]>
	<CA+TgmoaOSBQD9Ux4eG40w723ZN=c0J7p-+oX4+J8urUeyLMo5w@mail.gmail.com>
	<CAOYmi+=g+MMoOpWkk2weXWKJcKH0eKey8gKHHdH0dF4Tiawrhw@mail.gmail.com>
	<CA+TgmobwaT=PXPDDrgDup+jA8KHBbkxigtziD-zNzAKKkQYVgQ@mail.gmail.com>
	<CAOYmi+mOmEW=amDRQMfw6-Fb3ZmDEQFaJzwk8Bc8W8DzaP85XQ@mail.gmail.com>
	<CA+TgmoaX2AMW4cdFM3OngBJxmxpkdmzF33R7-CWhvRLfucbFMg@mail.gmail.com>
	<CAOYmi+njnRGcomnxTY6vsEW3wWigc0ruB0EyWFpb+PVVE8sWpw@mail.gmail.com>
	<CAOYmi+k4AyWCQHK=XVF99KVDuFkqxcADao61OWGLxu0nRYMONQ@mail.gmail.com>
	<CA+TgmoZ0x3ym_oueXRWzbM_=6ucKoPZVGj3rRMLBDC_FnetXDw@mail.gmail.com>
	<CAP53Pkycc=7N2bLzVT3x+qE1JamvRZWev5tFjdLJ1+-AV3Di+Q@mail.gmail.com>
	<CA+TgmoaKhuD91RnazbRyGkmP7--JdNq8oNDC3UcgTZSWbMxC7w@mail.gmail.com>
	<CAP53Pkw5-wMEeDJXFmqo_RTyL_spzCXb7HHKrbSnQqokVoZcNQ@mail.gmail.com>
	<CA+Tgmob-69bzbdi3U_QtebqAf6u1y8js=5=oNK639csVe1VbhA@mail.gmail.com>
	<CANWCAZZgKnf8dNOd_w03n88NqOfmMnMv2=D8_Oy6ADGyiMq+cg@mail.gmail.com>

On Tue, Jan 13, 2026 at 6:39 AM John Naylor <[email protected]> wrote:
> It looks like it would be helpful if fasthash_accum_cstring just
> returned zero when given a NULL string, as in the attached. We could
> also do something like add a large number to the hash, but I'm not
> sure that's necessary.

I think that pg_plan_advice's requirement are unusual here, so I would
suggest not adding a branch to fasthash_accum_cstring. If this were a
requirement that almost every caller had, then it would make sense to
pay the cost in the common function. But there will probably be a
small performance cost to this, hash function are often called in hot
paths, and pg_plan_advice is, I think, unusual, so I don't really like
doing it given thoe facts.

> + /*
> + * hashfn_unstable.h recommends using string length as tweak. It's not
> + * clear to me what to do if there are multiple strings, so for now I'm
> + * just using the total of all of the lengths.
> + */
> + return fasthash_final32(&hs, sp_len);
>
> Sounds reasonable, so the patch also documents that.

Some kind of comment change here seems useful to me. I wonder whether
it should be generalized even more than this statement. I also wonder
if this is really the optimal strategy. But I definitely agree that
clarifying this in whatever way makes sense is a good idea.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






view thread (143+ messages)  latest in thread

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pg_plan_advice
  In-Reply-To: <CA+Tgmoa6iQAz-RmAd3tWc=Rgr6beZDetZuA7o298tn=6prLhsA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox