Re: A performance issue with Memoize - Mailing list pgsql-hackers

From Richard Guo
Subject Re: A performance issue with Memoize
Date
Msg-id CAMbWs48d4JLv2uQsvK0EfqeNRACiyLT9yTUyu_4oSGxvKSR96g@mail.gmail.com
Whole thread Raw
In response to Re: A performance issue with Memoize  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers

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

pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Small fix on COPY ON_ERROR document
Next
From: Yugo NAGATA
Date:
Subject: Re: Small fix on COPY ON_ERROR document