On Wed, 14 Apr 2021 at 05:40, James Coleman <jtc...@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
v2-0001-Speedup-NOT-IN-with-a-set-of-Consts.patch
Description: Binary data
