From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Korotkov <aekorotkov@gmail.com>
cc: Alena Rybakina <a.rybakina@postgrespro.ru>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
        Andrei Lepikhov <lepihov@gmail.com>,
        Robert Haas <robertmhaas@gmail.com>,
        Richard Guo <guofenglinux@gmail.com>,
        Alexander Lakhin <exclusion@gmail.com>,
        "Gregory Stark (as CFM)" <stark.cfm@gmail.com>,
        =?UTF-8?B?TWljaGHFgiBLxYJlY3plaw==?= <michal@kleczek.org>,
        Dean Rasheed <dean.a.rasheed@gmail.com>
Subject: Re: Removing unneeded self joins
In-reply-to: 
 <CAPpHfdseDr02BT+kFfW1ORWd0xnf9B=OUNmYkDxF2x-bE32UOw@mail.gmail.com>
References: <64486b0b-0404-e39e-322d-0801154901f3@postgrespro.ru>
 <2422119.1714691974@sss.pgh.pa.us>
 <CAPpHfdv6B8HCLdj8WidBryRrX0+X3F1rrR8uAuMQmp6rvPdscg@mail.gmail.com>
 <24e93efd-eeac-43a6-a75a-ad68eacb32ee@postgrespro.ru>
 <CA+TgmoZ66x=uWasH+O40a=G=fpbXN1ANEdGVunvR8BKuekqa2g@mail.gmail.com>
 <3622801.1715010885@sss.pgh.pa.us>
 <CAPpHfduRB9oW3YRDpvPGcDSAbH4iu0LSRhxpRRAzBh7euC9amQ@mail.gmail.com>
 <CAEZATCUeJ93gWCEzK1eRxyZakvv5PVG9LfF3PkSGMsNBC7h9kA@mail.gmail.com>
 <CAPpHfdsj1WdqoCg+g4oq1XQ_=KOj0U4pXqhauBc7e+0msHu66A@mail.gmail.com>
 <CAPpHfds16a7bjQHv-jCLMNzwdukJ_i3da2e0OJFB9L5rTCWEEg@mail.gmail.com>
 <ac7d7334-7ebb-4330-a894-12ba12ffa4ae@gmail.com>
 <CAPpHfdsuc524-BkXNZ+iz9dH=-KiU8qx9CSyE-Nip4Wjz5pGgg@mail.gmail.com>
 <ac34e44e-f8e1-4601-8bb8-fa5e65578832@gmail.com>
 <CAPpHfdvHzExyROoxvVQz6JR4tGVOjgNyHnu8xJmGcx-bMxWvqw@mail.gmail.com>
 <5934ce1e-be02-4640-904c-f7fec3962784@gmail.com>
 <CAPpHfdt9Z=iriS1Fj0Vyo41Z4cS+zUSZM8D3qXyw5LfHr_jVjA@mail.gmail.com>
 <d6319b0c-b186-472f-8b64-41dfa4!
 1e7b5f@postgrespro.ru>
 <CAPpHfdsFcde5VWCxSup7x_12ig6uynrFaOms62-M_c8DM-FTBQ@mail.gmail.com>
 <CAPpHfdseDr02BT+kFfW1ORWd0xnf9B=OUNmYkDxF2x-bE32UOw@mail.gmail.com>
Comments: In-reply-to Alexander Korotkov <aekorotkov@gmail.com>
	message dated "Sat, 15 Feb 2025 11:19:42 +0200"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <914329.1740330169.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 23 Feb 2025 12:02:49 -0500
Message-ID: <914330.1740330169@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/914330.1740330169%40sss.pgh.pa.us>
Precedence: bulk

Alexander Korotkov <aekorotkov@gmail.com> writes:
> I've corrected some spelling error reported by Alexander Lakhin
> privately to me.  Also, I've revised comments around ChangeVarNodes()
> and ChangeVarNodesExtended().  I'm going to continue nitpicking this
> patch during next couple days then push it if no objections.

Coverity has another nit-pick:

/srv/coverity/git/pgsql-git/postgresql/src/backend/optimizer/plan/analyzej=
oins.c: 327 in remove_rel_from_query()
321     static void
322     remove_rel_from_query(PlannerInfo *root, RelOptInfo *rel,
323     					  int subst, SpecialJoinInfo *sjinfo,
324     					  Relids joinrelids)
325     {
326     	int			relid =3D rel->relid;
>>>     CID 1643155:  Integer handling issues  (INTEGER_OVERFLOW)
>>>     Expression "ojrelid", which is equal to 4294967295, where "(sjinfo=
 !=3D NULL) ? sjinfo->ojrelid : 4294967295U" is known to be equal to 42949=
67295, overflows the type that receives it, a signed integer 32 bits wide.
327     	int			ojrelid =3D (sjinfo !=3D NULL) ? sjinfo->ojrelid : -1;
328     	Index		rti;
329     	ListCell   *l;
330     =

331     	/*
332     	 * Update all_baserels and related relid sets.

This is unhappy because sjinfo->ojrelid, which is declared Index
(that is, unsigned int) is being converted to int.  Admittedly
there are plenty of other places that do likewise, but the additional
load of assuming that -1 isn't a possible value of sjinfo->ojrelid
seems to be enough to draw its ire.

I suggest finding another way to code this function that doesn't
depend on that type pun.  I think it's fairly accidental that
adjust_relid_set doesn't misbehave on -1 anyway, so personally I'd
get rid of that local variable entirely in favor of something like

    if (sjinfo !=3D NULL)
    {
        root->outer_join_rels =3D adjust_relid_set(root->outer_join_rels,
                                                 sjinfo->ojrelid, subst);
        root->all_query_rels =3D adjust_relid_set(root->all_query_rels,
                                                sjinfo->ojrelid, subst);
    }

			regards, tom lane