On Wed, 14 Apr 2021 at 05:40, James Coleman <jtc331@gmail.com> wrote:
> ...and here's a draft patch. I can take this to a new thread if you'd
> prefer; the one here already got committed, on the other hand this is
> pretty strongly linked to this discussion, so I figured it made sense
> to post it here.
I only glanced at this when you sent it and I was confused about how
it works. The patch didn't look like how I imagined it should and I
couldn't see how the executor part worked without any changes.
Anyway, I decided to clear up my confusion tonight and apply the patch
to figure all this out... unfortunately, I see why I was confused
now. It actually does not work at all :-(
You're still passing the <> operator to get_op_hash_functions(), which
of course is not hashable, so we just never do hashing for NOT IN.
All your tests pass just fine because the standard non-hashed code path is used.
My idea was that you'd not add any fields to ScalarArrayOpExpr and for
soaps with useOr == false, check if the negator of the operator is
hashable. If so set the opfuncid to the negator operator's function.
I'm a bit undecided if it's safe to set the opfuncid to the negator
function. If anything were to set that again based on the opno then
it would likely set it to the wrong thing. We can't go changing the
opno either because EXPLAIN would display the wrong thing.
Anyway, I've attached what I ended up with after spending a few hours
looking at this.
I pretty much used all your tests as is with the exception of removing
one that looked duplicated.
David