Re: A performance issue with Memoize

On Fri, Jan 26, 2024 at 1:22 AM Tom Lane <tgl@sss.pgh.pa.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