Re: A performance issue with Memoize

On Fri, Jan 26, 2024 at 1:22 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> 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;
> + }
> }
> }

Yeah, I think it should be safe to assert that the varnullingrels is
equal here. The Var is supposed to be an upper-level Var, and two same
such Vars should not have different varnullingrels at this point,
although the varnullingrels might be adjusted later in
identify_current_nestloop_params according to which form of identity 3
we end up applying.

Thanks
Richard