From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <dgrowleyml@gmail.com>
cc: Richard Guo <guofenglinux@gmail.com>,
        PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: A performance issue with Memoize
In-reply-to: 
 <CAApHDvoTyLG35Rm26F7-MFmX3kv-uAs9FD4XF7fqy-B0W6o_UQ@mail.gmail.com>
References: 
 <CAMbWs48XHJEK1Q1CzAQ7L9sTANTs9W1cepXu8=Kc0quUL+tg4Q@mail.gmail.com>
 <CAApHDvoTyLG35Rm26F7-MFmX3kv-uAs9FD4XF7fqy-B0W6o_UQ@mail.gmail.com>
Comments: In-reply-to David Rowley <dgrowleyml@gmail.com>
	message dated "Thu, 25 Jan 2024 13:13:41 +1300"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <412551.1706203379.1@sss.pgh.pa.us>
Date: Thu, 25 Jan 2024 12:22:59 -0500
Message-ID: <412552.1706203379@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/412552.1706203379%40sss.pgh.pa.us>
Precedence: bulk

David Rowley <dgrowleyml@gmail.com> writes:
> I'd feel better about doing it your way if Tom could comment on if
> there was a reason he put the function calls that way around in
> 5ebaaa494.

Apologies for not having noticed this thread before.  I'm taking
a look at it now.  However, while sniffing around this I found
what seems like an oversight in paramassign.c's
assign_param_for_var(): it says it should compare all the same
fields as _equalVar except for varlevelsup, but it's failing to
compare varnullingrels.  Is that a bug?  It's conceivable that
it's not possible to get here with varnullingrels different and
all else the same, but I don't feel good about that proposition.

I tried adding

@@ -91,7 +91,10 @@ assign_param_for_var(PlannerInfo *root, Var *var)
                 pvar->vartype == var->vartype &&
                 pvar->vartypmod == var->vartypmod &&
                 pvar->varcollid == var->varcollid)
+            {
+                Assert(bms_equal(pvar->varnullingrels, var->varnullingrels));
                 return pitem->paramId;
+            }
         }
     }

This triggers no failures in the regression tests, but we know
how little that proves.

Anyway, that's just a side observation unrelated to the problem
at hand.  More later.

			regards, tom lane