PG19: flat-copied RelOptInfos share extension_state with the original rel
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t254154psql -h localhost -U postgresBuilt from patchset v5 (message #5), October 22, 2026 at 11:10 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t254154_5 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t254154_5 && git checkout t254154_5Patchset v5 (message #5) is on t254154_5
Hi hackers,
While testing the new v19 planner extension APIs with an extension that
keeps per-rel state, I found that RelOptInfos created by flat copy
share their extension_state array with the rel they were copied from.
build_grouped_rel(), create_unique_paths() and
create_partial_unique_paths() memcpy() a RelOptInfo and reset the path
fields, and build_minmax_path() does the same for PlannerInfo, but none
of them resets extension_state / extension_state_allocated. With
joinrel_setup_hook (4020b370f2) and build_simple_rel_hook (91f33a2ae9),
the natural place for an extension to attach state is right when a rel
is built, and make_join_rel() runs joinrel_setup_hook before
make_grouped_join_rel() copies the joinrel. So:
1. When join_path_setup_hook or set_join_pathlist_hook is called for
the grouped joinrel, GetRelOptInfoExtensionState() returns the state
stored for the plain joinrel, and anything stored for the grouped
joinrel overwrites the plain joinrel's entry.
2. If the store needs a slot past the end of the array,
SetRelOptInfoExtensionState() repallocs the shared array. That
frees the array the plain joinrel still points to, and the next
lookup on the plain joinrel reads freed memory.
Eager aggregation is on by default, so a GROUP BY over a two-table join
is enough. The attached test module stores a pointer to the owning rel
in its state, so it can tell whose state it gets back. On
REL_19_STABLE (2359968b44) with --enable-cassert, using the tables from
eager_aggregate.sql:
SET test_extstate.mode = 1;
EXPLAIN (COSTS OFF)
SELECT t1.a, avg(t2.c)
FROM eager_agg_t1 t1 JOIN eager_agg_t2 t2 ON t1.b = t2.b
GROUP BY t1.a;
WARNING: extension state on grouped joinrel (b 1 2) belongs to a different RelOptInfo
SET test_extstate.mode = 2; -- also stores into a higher slot
EXPLAIN ... same query ...
WARNING: plain joinrel (b 1 2) still points at array 0x740a72cbfc48, which repalloc just freed; slot 1 now reads 0x7f7f7f7f7f7f7f7f
server closed the connection unexpectedly.
The crash is in the module's join_path_setup_hook, called from
add_paths_to_joinrel() <- populate_joinrel_with_paths() <- make_join_rel() for the plain joinrel.
With enable_eager_aggregate = off there are no warnings.
This is the same pattern as 58fca6bdf2 ("Don't pass grouped relations to FDWs"), which cleared the FDW fields in build_grouped_rel() [1]/messages/by-id/CA+Tgmob7iSM9YkRM44VjUDuaCchW-fY54MV5njpTZTL9uNyV4w@mail.gmail.com;
extension_state needs the same treatment there and at the other copy
sites. For the unique-ified rel, the intent was that it stays opaque
to extensions [2]/messages/by-id/CAMbWs4-UXdxMaZO-GgNx7Y4tWoDutc4uVmnhUBn+8afHp520og@mail.gmail.com, but it is passed to the join hooks as outerrel /innerrel, so extensions do see it.
The attached patch resets the two fields at all four copy sites. The
build_minmax_path() copy is not reachable through any current hook,
because preprocess_minmax_aggregates() runs before anything can attach
state to the parent PlannerInfo, but resetting it there seems right
too. The patch also adds the "constant string" note that
GetExplainExtensionId() already has to GetPlannerExtensionId(), since
it keeps the caller's pointer rather than copying the name.
With the patch, the reproducer shows no warnings, the plan still has
the Partial HashAggregate below the join, and the core regression
tests pass (installcheck-parallel, 239 tests, cassert + ASan build).
For the eager-deduplication series [3]/messages/by-id/PH0PR18MB444315CD3E17255AB4A629BEA6AD2@PH0PR18MB4443.namprd18.prod.outlook.com: its proposed
copy_rel_without_paths() would be a good single place to reset
extension_state, if that lands.
I found this while reviewing the v19 extension-facing planner APIs with
the help of an AI code-review and confirmed it on the build
above.
[1]: /messages/by-id/CA+Tgmob7iSM9YkRM44VjUDuaCchW-fY54MV5njpTZTL9uNyV4w@mail.gmail.com
[2]: /messages/by-id/CAMbWs4-UXdxMaZO-GgNx7Y4tWoDutc4uVmnhUBn+8afHp520og@mail.gmail.com
[3]: /messages/by-id/PH0PR18MB444315CD3E17255AB4A629BEA6AD2@PH0PR18MB4443.namprd18.prod.outlook.com
Regards,
Shubhankar K.
Member Technical Staff
Zoho Corporation
Import Notes
Reply to msg id not found:
On Mon, Oct 12, 2026 at 6:41 AM Shubhankar Anand Kulkarni <
shubhankar.ak@zohocorp.com> wrote:
The attached patch resets the two fields at all four copy sites.
Hi Shubhankar,
I reviewed v1 on master. All core regression tests pass.
Before the patch, I reproduced the state-sharing warnings in
mode 1 and the backend crash in mode 2. Both modes complete without
warnings after the patch. I found no blocking issues.
Thanks,
Sho Ito
--
=======================================
*Sho Ito (Sean)*
E-mail: i.sho.0628@gmail.com
LinkedIn: *https://www.linkedin.com/in/sho-ito-0628/
<https://www.linkedin.com/in/sho-ito-0628/>*
GitHub: https://github.com/sean0628/
=======================================
On Tue, Oct 13, 2026 Sho Ito < mailto:i.sho.0628@gmail.com > wrote:
I reviewed v1 on master. All core regression tests pass.
Before the patch, I reproduced the state-sharing warnings in
mode 1 and the backend crash in mode 2. Both modes complete without
warnings after the patch. I found no blocking issues.
Hi Sho,
Thanks for the review, and for reproducing it independently on master.
The commit-fest entry is https://commitfest.postgresql.org/patch/7436/ ,
and cfbot is green on all platforms. If you're satisfied with it,
could you mark it Ready for Committer?
Since this touches both the extension-state API and
eager aggregation, are you OK with resetting the fields at each copy
site, or would you rather have a shared helper that does the flat copy
and the resets in one place?
Also can we turn the reproducer into a src/test/modules test if you'd like regression coverage for it?
Regards,l
Shubhankar K.
Member Technical Staff
Zoho Corporation
From: Sho Ito <i.sho.0628@gmail.com>
To: "Shubhankar Anand Kulkarni"<shubhankar.ak@zohocorp.com>
Cc: "pgsql-hackers"<pgsql-hackers@lists.postgresql.org>, "pgsql-hackers"<pgsql-hackers@postgresql.org>, "robertmhaas"<robertmhaas@gmail.com>, "guofenglinux"<guofenglinux@gmail.com>
Date: Tue, 13 Oct 2026 04:01:52 +0530
Subject: Re: PG19: flat-copied RelOptInfos share extension_state with the original rel
On Mon, Oct 12, 2026 at 6:41 AM Shubhankar Anand Kulkarni < mailto:shubhankar.ak@zohocorp.com > wrote:
The attached patch resets the two fields at all four copy sites.
Hi Shubhankar,
I reviewed v1 on master. All core regression tests pass.
Before the patch, I reproduced the state-sharing warnings in
mode 1 and the backend crash in mode 2. Both modes complete without
warnings after the patch. I found no blocking issues.
Thanks,
Sho Ito
--
=======================================
Sho Ito (Sean)
E-mail: mailto:i.sho.0628@gmail.com
LinkedIn: https://www.linkedin.com/in/sho-ito-0628/
GitHub: https://github.com/sean0628/
=======================================
On Tue, Oct 13, 2026 at 6:37 AM Shubhankar Anand Kulkarni <
shubhankar.ak@zohocorp.com> wrote:
Since this touches both the extension-state API and
eager aggregation, are you OK with resetting the fields at each copy
site, or would you rather have a shared helper that does the flat copy
and the resets in one place?
I'm okay with the per-site resets for this fix, alongside the existing
duplicated path resets.
Also can we turn the reproducer into a src/test/modules test if you'd like
regression coverage for it?
Yes, I believe regression coverage under src/test/modules would be useful.
I'll hold off on updating the status until I've reviewed the
revised patch and run the new test.
Thanks,
Sho Ito
--
=======================================
*Sho Ito (Sean)*
E-mail: i.sho.0628@gmail.com
LinkedIn: *https://www.linkedin.com/in/sho-ito-0628/
<https://www.linkedin.com/in/sho-ito-0628/>*
GitHub: https://github.com/sean0628/
=======================================
On Wed, Oct 14, 2026 Sho Ito < mailto:i.sho.0628@gmail.com > wrote:
I'm okay with the per-site resets for this fix, alongside the existing
duplicated path resets.
Yes, I believe regression coverage under src/test/modules would be useful.
Hi Sho,
Thanks. Attached is v2.
v2-0001 is unchanged from v1 apart from its commit message, which now
carries the usual Author, Reviewed-by and Discussion lines.
v2-0002 adds src/test/modules/test_extendplan. The module stores a tag in
the extension state of every base and join rel as the planner builds it
(build_simple_rel_hook, joinrel_setup_hook), recording which RelOptInfo
it was stored in. join_path_setup_hook then checks the joinrel and both
input rels, and emits a WARNING if any of them holds a tag stored in a
different RelOptInfo. The test plans an eager-aggregation query and an
IN semijoin. test_extendplan_summary() also reports whether a grouped
rel and a unique-ified rel were actually checked, so the test can't
pass vacuously if the planner stops building those rels for these
queries.
With 0001 and 0002 the test passes. With 0002 alone it fails:
+WARNING: extension state of grouped joinrel (b 1 2) belongs to a different RelOptInfo
+WARNING: extension state of grouped inner rel (b 2) belongs to a different RelOptInfo
...
+WARNING: extension state of inner rel (b 3) belongs to a different RelOptInfo
+WARNING: extension state of outer rel (b 3) belongs to a different RelOptInfo
That covers build_grouped_rel(), for both base and join rels, and
create_unique_paths(). The copies made in create_partial_unique_paths()
and build_minmax_path() never reach an extension hook, so the test
can't see them; 0001 resets them for consistency.
The test checks which rel a tag belongs to rather than provoking the
use-after-free. Whether reading the freed array crashes depends on the
build: with assertions enabled the freed memory is clobbered and the
backend crashes, but without them the stale pointer is usually still
there and the lookup appears to work. A crash-based test would
therefore not fail reliably without 0001, whereas the ownership check
fails on every build.
I'll hold off on updating the status until I've reviewed the
revised patch and run the new test.
Once you verify the test behaviour and it satisfies the need, we can update the patch status accordingly.
Regards,
Shubhankar K.
Member Technical Staff
Zoho Corporation
From: Sho Ito <i.sho.0628@gmail.com>
To: "Shubhankar Anand Kulkarni"<shubhankar.ak@zohocorp.com>
Cc: "pgsql-hackers"<pgsql-hackers@lists.postgresql.org>, "pgsql-hackers"<pgsql-hackers@postgresql.org>, "robertmhaas"<robertmhaas@gmail.com>, "guofenglinux"<guofenglinux@gmail.com>
Date: Wed, 14 Oct 2026 09:02:06 +0530
Subject: Re: PG19: flat-copied RelOptInfos share extension_state with the original rel
On Tue, Oct 13, 2026 at 6:37 AM Shubhankar Anand Kulkarni < mailto:shubhankar.ak@zohocorp.com > wrote:
Since this touches both the extension-state API and
eager aggregation, are you OK with resetting the fields at each copy
site, or would you rather have a shared helper that does the flat copy
and the resets in one place?
I'm okay with the per-site resets for this fix, alongside the existing
duplicated path resets.
Also can we turn the reproducer into a src/test/modules test if you'd like regression coverage for it?
Yes, I believe regression coverage under src/test/modules would be useful.
I'll hold off on updating the status until I've reviewed the
revised patch and run the new test.
Thanks,
Sho Ito
--
=======================================
Sho Ito (Sean)
E-mail: mailto:i.sho.0628@gmail.com
LinkedIn: https://www.linkedin.com/in/sho-ito-0628/
GitHub: https://github.com/sean0628/
=======================================
Attachments:
t254154_5v2-0002-Add-a-test-module-for-planner-extension-state.patchapplication/octet-stream; name=v2-0002-Add-a-test-module-for-planner-extension-state.patchDownload+352-1
v2-0001-Don-t-let-flat-copied-planner-structures-share-ex.patchapplication/octet-stream; name=v2-0001-Don-t-let-flat-copied-planner-structures-share-ex.patchDownload+25-1
On Wed, Oct 14, 2026 at 7:26 PM Shubhankar Anand Kulkarni <
shubhankar.ak@zohocorp.com> wrote:
The test checks which rel a tag belongs to rather than provoking the
use-after-free.
Hi Shubhankar,
Thanks for the update, and sorry for the late reply.
I reviewed v2 with assertions enabled. The test_extendplan
regression test passes with both patches. With 0002 alone, it fails with
ownership warnings; with 0001 applied, both summary checks report zero
mismatches.
I found no blocking issues and think it is ready for committer review. I've
updated the status.
Thanks,
Sho Ito
--
=======================================
*Sho Ito (Sean)*
E-mail: i.sho.0628@gmail.com
LinkedIn: *https://www.linkedin.com/in/sho-ito-0628/
<https://www.linkedin.com/in/sho-ito-0628/>*
GitHub: https://github.com/sean0628/
=======================================