PG19: flat-copied RelOptInfos share extension_state with the original rel

Started by Shubhankar Anand Kulkarni10 days ago7 messageshackers
Beta feature

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.

appliessuccessCI history

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:t254154
psql -h localhost -U postgres

Built 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.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t254154_5 && git checkout t254154_5

Patchset v5 (message #5) is on t254154_5

Jump to latest
#1Shubhankar Anand Kulkarni
shubhankar.ak@zohocorp.com

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

Attachments:

t254154_1
repro.sqlapplication/octet-stream; name=repro.sqlDownload
test_extstate.capplication/octet-stream; name=test_extstate.cDownload
v1-0001-Don-t-let-flat-copied-planner-structures-share-ex.patchapplication/octet-stream; name=v1-0001-Don-t-let-flat-copied-planner-structures-share-ex.patchDownload+25-1
asan_crash_report_stock.txttext/plain; name=asan_crash_report_stock.txtDownload
#2Sho Ito
i.sho.0628@gmail.com
In reply to: Shubhankar Anand Kulkarni (#1)
Re: PG19: flat-copied RelOptInfos share extension_state with the original rel

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/&gt;*
GitHub: https://github.com/sean0628/
=======================================

#3Shubhankar Anand Kulkarni
shubhankar.ak@zohocorp.com
In reply to: Sho Ito (#2)
Re: PG19: flat-copied RelOptInfos share extension_state with the original rel

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/

=======================================

#4Sho Ito
i.sho.0628@gmail.com
In reply to: Shubhankar Anand Kulkarni (#3)
Re: PG19: flat-copied RelOptInfos share extension_state with the original rel

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/&gt;*
GitHub: https://github.com/sean0628/
=======================================

#5Shubhankar Anand Kulkarni
shubhankar.ak@zohocorp.com
In reply to: Sho Ito (#4)
Re: PG19: flat-copied RelOptInfos share extension_state with the original rel

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_5
v2-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
#6Sho Ito
i.sho.0628@gmail.com
In reply to: Shubhankar Anand Kulkarni (#5)
Re: PG19: flat-copied RelOptInfos share extension_state with the original rel

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/&gt;*
GitHub: https://github.com/sean0628/
=======================================

#7Shubhankar Anand Kulkarni
shubhankar.ak@zohocorp.com
In reply to: Sho Ito (#6)
Re: PG19: flat-copied RelOptInfos share extension_state with the original rel

Hi Sho,

No problem at all, and thanks for reviewing both versions and for running the new test.

Regards,

Shubhankar K
Member Technical Staff
Zoho Corporation