MIME-Version: 1.0
References: <19355-57d7d52ea4980dc6@postgresql.org> <868ff2a518820c8864b6d28510294b2457a126af.camel@cybertec.at>
 <CANjBVm3AMm=9-mB7=uqJjaWN-+RrA8+w-gXhZn6L7VHu7K_Urw@mail.gmail.com>
 <CAEZATCVcV4cfqGnmPGVxwjsPh7rxsTD510=KOGFyi2_FMgqWaQ@mail.gmail.com>
 <CA+HiwqE8gNK7Utqo2EAKh24KExWfypt06m1EFNXBd5TFDO2f6w@mail.gmail.com>
 <CAHewXNn8ApcYfZKjZ1R8Z7yHRoi+C+mu5r9LbZUbb6691d6Rrw@mail.gmail.com>
 <CAEZATCVr+=F=iTbi3XyABSEr5Cqw=ptm2PyGc75SaQ1JYrPnsg@mail.gmail.com>
 <CAHewXN=D2SUh1CvE0gnk1Ev_k4RAEO9aoEgQZG_7eAuSWxZA0Q@mail.gmail.com>
 <CAEDh4nw=h5NihhuBgq4GFCfPP9vcg54QJ+K0XsG9S0owWDUhfA@mail.gmail.com> <CAEDh4nzHw6LciY1A556ybfNB_TmKNa-gSTJAKvEMVi=zJZ8Yig@mail.gmail.com>
In-Reply-To: <CAEDh4nzHw6LciY1A556ybfNB_TmKNa-gSTJAKvEMVi=zJZ8Yig@mail.gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Wed, 7 Jan 2026 18:45:01 +0900
Message-ID: <CA+HiwqGpeG2jo51o6XoXfFYpoXGg=NM2Byj0Yjk7G1rzr=0mJw@mail.gmail.com>
Subject: Re: BUG #19355: Attempt to insert data unexpectedly during concurrent update
To: Bernice Southey <bernice.southey@gmail.com>
Cc: Tender Wang <tndrwang@gmail.com>, Dean Rasheed <dean.a.rasheed@gmail.com>, 
	Bh W <wangbihua.cn@gmail.com>, Laurenz Albe <laurenz.albe@cybertec.at>, 
	pgsql-bugs@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CA%2BHiwqGpeG2jo51o6XoXfFYpoXGg%3DNM2Byj0Yjk7G1rzr%3D0mJw%40mail.gmail.com>
Precedence: bulk

Hi Bernice,

On Fri, Jan 2, 2026 at 6:58=E2=80=AFAM Bernice Southey
<bernice.southey@gmail.com> wrote:
> > > Dean Rasheed wrote:
> > >> I'm somewhat conflicted as to which approach is better. I think mayb=
e
> > >> there is less chance of other unintended side-effects if the set of
> > >> RTEs included in PlannerGlobal.allRelids, unprunableRelids, and
> > >> es_unpruned_relids is not changed. However, as it stands,
> > >> PlannerGlobal.allRelids is misnamed (it should probably have been
> > >> called "relationRelids", in line with the "relationOids" field).
> > >> Making it actually include all RTEs would solve that.
>
> I did some more digging (postgres source code is addictive).
>
> Up until v56-0004-Defer-locking-of-runtime-prunable-relations-to-e.patch
> [1], the existing unfiltered rtable is used to create
> unprunableRelids.
>
> +++ b/src/backend/optimizer/plan/planner.c
> @@ -555,6 +555,8 @@ standard_planner(Query *parse, const char
> *query_string, int cursorOptions,
>   result->planTree =3D top_plan;
>   result->partPruneInfos =3D glob->partPruneInfos;
>   result->rtable =3D glob->finalrtable;
> + result->unprunableRelids =3D bms_difference(bms_add_range(NULL, 1,
> list_length(result->rtable)),
> +   glob->prunableRelids);
>
> In v56 [2], the filtered allRelids was added. I think this is when the
> bug was introduced, because the three places from Dean's patch are in
> both versions.
>
> I've looked much harder at the code (I'm still at
> stumbling-around-in-the-dark-with-a-match level) and AFAICT, the two
> approaches are very similar. I think equal effort is required to check
> that PlannerGlobal.allRelids, unprunableRelids, and es_unpruned_relids
> are correct, whichever approach is used. I can't find any missed cases
> in either approach - with my matchlight.

Good catch on the history -- that's exactly when the bug was
introduced. I'd say Dean's approach is easier to verify since it's a
simple check at the point of use, rather than ensuring allRelids is
built correctly across all planner code paths.

> Sorry for my ignorance: does a relId refer to a range table index and
> a relation to a...for lack of a better word...table+?

Depends on the context, but "relid" in the planner internal data
structures refer to RT index.  In the planner output data structures
(plan nodes, etc.), we typically use "rti" or "rtindex".

--=20
Thanks, Amit Langote