public inbox for [email protected]  

help / color / mirror / Atom feed

From: Chengpeng Yan <[email protected]>
To: David Rowley <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: cca5507 <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: [PATCH] Fix hashed ScalarArrayOp semantics for NULL LHS with non-strict comparators
Date: Thu, 23 Apr 2026 04:31:24 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAApHDvo=bOVURpRH_ydBYT_2R9PU4EvzzFfez1xV4RKBPRUPVA@mail.gmail.com>
References: <[email protected]>
	<CAApHDvoLFhtAFSwJdjsBcs9L4J4SzE09gkHLr9nJSGSj1CBaRw@mail.gmail.com>
	<CAApHDvr7K+Wu+ECgmZw057Mb1H6H87+ZWETHi28Ez_ALBKgmQA@mail.gmail.com>
	<[email protected]>
	<CAApHDvo=bOVURpRH_ydBYT_2R9PU4EvzzFfez1xV4RKBPRUPVA@mail.gmail.com>

Hi,

> On Apr 23, 2026, at 07:33, David Rowley <[email protected]> wrote:
> 
> Yeah, it doesn't make sense to repeatedly perform a linear search over
> the array to check if NULL matches anything in the array. Let's just
> do that once when we build the hash table and reuse that cached value
> whenever we see a NULL. We can skip that step with strict functions
> since we'll short-circuit earlier.
> 
> A patch for that is attached.

Thanks for working on this. Overall, this version looks good to me, and
I'm fine with the current approach. One possible improvement, though not
a blocker, would be to defer the lhs-NULL handling until we actually
encounter the first NULL on the lhs. That could avoid a bit of extra
work in the common case where the lhs contains no NULLs. That said, I
think the current implementation is perfectly OK as-is.

> IMO it's unrealistic to assume we can do anything sane with an
> equality function that always returns NULL.
> 
> I really doubt it's worth troubling over that. If we did want to do
> something, then it would be more efficient to probe the hash table
> directly after we insert a Datum and verify we can find it again. If
> we can't find any value we just inserted, mark the entire table as
> broken and have it so we check for that and do a linear search.

I tend to agree. Even if such a case can be constructed, it seems rare
enough that I am not sure it is worth adding more complexity, or extra
overhead in the common hashed SAOP path, to handle it in this patch. I
think we can revisit that separately if a concrete case turns up that
seems worth looking into.

--
Best regards,
Chengpeng Yan

view thread (11+ 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]
  Subject: Re: [PATCH] Fix hashed ScalarArrayOp semantics for NULL LHS with non-strict comparators
  In-Reply-To: <[email protected]>

* 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