MIME-Version: 1.0
References: <19493-5878eac7a2525c23@postgresql.org>
 <2d59d7d6-6afe-4565-8ff7-ae764651589a@gmail.com>
 <ahY_s6GLOGoceXHT@paquier.xyz> <1779843554.737126560@fmail2.qdit>
 <418106.1779844099@sss.pgh.pa.us>
 <CAHewXNn7q9Bm=i=ZBMJxUsmFhavfdxwbwVzdhsV9uXZOk584HQ@mail.gmail.com>
 <CAHewXN=n_KzrOFgHyZwSHPuaXF-RRMig0o4yL+knoSE-_cMMPA@mail.gmail.com>
 <CAHewXNkHsjOjaWUcdtrGTWeHK8f1N8=L434O0b9ecgtGaMFQrg@mail.gmail.com>
In-Reply-To: 
 <CAHewXNkHsjOjaWUcdtrGTWeHK8f1N8=L434O0b9ecgtGaMFQrg@mail.gmail.com>
From: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Date: Wed, 27 May 2026 12:46:24 +0530
Message-ID: 
 <CAJTYsWUsW6zm_5thDsmV=c9VLs-O==r1RusDDF7kCekV0AECDA@mail.gmail.com>
Subject: Re: BUG #19493: Assertion failure in pg_plan_advice with EXISTS
 subquery and DO_NOT_SCAN advice
To: Tender Wang <tndrwang@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org,
	=?UTF-8?B?0JrQsNC70LjQvdC40L0g0J3QuNC60LjRgtCw?= <n.kalinin@postgrespro.ru>,
	Michael Paquier <michael@paquier.xyz>,
 Pierre Forstmann <pierre.forstmann@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>
Content-Type: multipart/alternative; boundary="0000000000002bd5170652c76491"
Archived-At: 
 <https://www.postgresql.org/message-id/CAJTYsWUsW6zm_5thDsmV%3Dc9VLs-O%3D%3Dr1RusDDF7kCekV0AECDA%40mail.gmail.com>
Precedence: bulk

--0000000000002bd5170652c76491
Content-Type: text/plain; charset="UTF-8"

Hi,

On Wed, 27 May 2026 at 09:20, Tender Wang <tndrwang@gmail.com> wrote:

> Hi, all
>
> I find an easier way as follows:
> diff --git a/contrib/pg_plan_advice/pgpa_trove.c
> b/contrib/pg_plan_advice/pgpa_trove.c
> index ca69f3bd3df..64af4b1435b 100644
> --- a/contrib/pg_plan_advice/pgpa_trove.c
> +++ b/contrib/pg_plan_advice/pgpa_trove.c
> @@ -179,7 +179,6 @@ pgpa_build_trove(List *advice_items)
>                                          * but in the future this
> might not be true, e.g. a custom
>                                          * scan could replace a join.
>                                          */
> -                                       Assert(target->ttype ==
> PGPA_TARGET_IDENTIFIER);
>
> pgpa_trove_add_to_slice(&trove->scan,
>
>                  item->tag, target);
>                                 }
>

Thanks for checking this.

I agree that removing the assertion looks like the better approach.

Keeping the original target tree seems preferable.  As you noted,
pgpa_identifier_matches_target() already handles non-identifier targets by
checking their descendants.  pgpa_trove_add_to_hash() does the same when
building the lookup table, so a grouped target such as ((a)) should still be
indexed and matched through its child identifier while preserving the
original
shape for output.

So I think the assertion in pgpa_build_trove() is too strict, and the nearby
comment should be adjusted to avoid saying/implying that scan advice always
has a direct identifier target.

Regards,
Ayush

--0000000000002bd5170652c76491
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi,</div><br><div class=3D"gmail_quote gm=
ail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On Wed, 27 May 2=
026 at 09:20, Tender Wang &lt;<a href=3D"mailto:tndrwang@gmail.com">tndrwan=
g@gmail.com</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=
=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding=
-left:1ex">Hi, all<br>
<br>
I find an easier way as follows:<br>
diff --git a/contrib/pg_plan_advice/pgpa_trove.c<br>
b/contrib/pg_plan_advice/pgpa_trove.c<br>
index ca69f3bd3df..64af4b1435b 100644<br>
--- a/contrib/pg_plan_advice/pgpa_trove.c<br>
+++ b/contrib/pg_plan_advice/pgpa_trove.c<br>
@@ -179,7 +179,6 @@ pgpa_build_trove(List *advice_items)<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* =
but in the future this<br>
might not be true, e.g. a custom<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* =
scan could replace a join.<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/=
<br>
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Assert(ta=
rget-&gt;ttype =3D=3D<br>
PGPA_TARGET_IDENTIFIER);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pgpa_tro=
ve_add_to_slice(&amp;trove-&gt;scan,<br>
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0item-&gt;tag,=
 target);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }<br></blockquote><div><br></div>Tha=
nks for checking this.<br><br>I agree that removing the assertion looks lik=
e the better approach.=C2=A0</div><div class=3D"gmail_quote gmail_quote_con=
tainer"><br>Keeping the original target tree seems preferable.=C2=A0 As you=
 noted,<br>pgpa_identifier_matches_target() already handles non-identifier =
targets by<br>checking their descendants. =C2=A0pgpa_trove_add_to_hash() do=
es the same when<br>building the lookup table, so a grouped target such as =
((a)) should still be<br>indexed and matched through its child identifier w=
hile preserving the original<br>shape for output.<br><br>So I think the ass=
ertion in pgpa_build_trove() is too strict, and the nearby<br>comment shoul=
d be adjusted to avoid saying/implying that scan advice always<br>has a dir=
ect identifier target.<br><br>Regards,<br><div>Ayush=C2=A0</div></div></div=
>

--0000000000002bd5170652c76491--