On Fri, Nov 24, 2023 at 7:05 AM Alena Rybakina <a.rybak...@postgrespro.ru> wrote: > On 23.11.2023 12:23, Andrei Lepikhov wrote: > > I think the usage of nodeToString for the generation of clause hash is > > too expensive and buggy. > > Also, in the code, you didn't resolve hash collisions. So, I've > > rewritten the patch a bit (see the attachment). > > One more thing: I propose to enable transformation by default at least > > for quick detection of possible issues. > > This code changes tests in many places. But, as I see it, it mostly > > demonstrates the positive effect of the transformation. > > On 24.11.2023 06:30, Andrei Lepikhov wrote: > > > On 23/11/2023 16:23, Andrei Lepikhov wrote: > >> This code changes tests in many places. But, as I see it, it mostly > >> demonstrates the positive effect of the transformation. > > > > I found out that the postgres_fdw tests were impacted by the feature. > > Fix it, because the patch is on the commitfest and passes buildfarm. > > Taking advantage of this, I suppressed the expression evaluation > > procedure to make regression test changes more clear. > > Thank you for your work. You are right, the patch with the current > changes looks better and works more correctly. > > To be honest, I didn't think we could use JumbleExpr in this way.
I think patch certainly gets better in this aspect. One thing I can't understand is why do we use home-grown code for resolving hash-collisions. You can just define custom hash and match functions in HASHCTL. Even if we need to avoid repeated JumbleExpr calls, we still can save pre-calculated hash value into hash entry and use custom hash and match. This doesn't imply us to write our own collision-resolving code. ------ Regards, Alexander Korotkov