Re: pg_plan_advice

On Tue, Jan 13, 2026 at 6:39 AM John Naylor <johncnaylorls(at)gmail(dot)com> 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