public inbox for [email protected]  

help / color / mirror / Atom feed

From: Richard Guo <[email protected]>
To: Tom Lane <[email protected]>
Cc: David Rowley <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: A performance issue with Memoize
Date: Fri, 26 Jan 2024 13:38:53 +0800
Message-ID: <CAMbWs48d4JLv2uQsvK0EfqeNRACiyLT9yTUyu_4oSGxvKSR96g@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAMbWs48XHJEK1Q1CzAQ7L9sTANTs9W1cepXu8=Kc0quUL+tg4Q@mail.gmail.com>
	<CAApHDvoTyLG35Rm26F7-MFmX3kv-uAs9FD4XF7fqy-B0W6o_UQ@mail.gmail.com>
	<[email protected]>

On Fri, Jan 26, 2024 at 1:22 AM Tom Lane <[email protected]> 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

view thread (20+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: A performance issue with Memoize
  In-Reply-To: <CAMbWs48d4JLv2uQsvK0EfqeNRACiyLT9yTUyu_4oSGxvKSR96g@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox