On Fri, Nov 24, 2023 at 7:05 AM Alena Rybakina
<a.rybakina@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