ERROR: too late to create a new PlaceHolderInfo

Started by Richard Guoabout 15 hours ago4 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:t253816
psql -h localhost -U postgres

Built from patchset v2 (message #2), September 17, 2026 at 05:43 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 t253816_2 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 t253816_2 && git checkout t253816_2

Patchset v2 (message #2) is on t253816_2

Jump to latest
#1Richard Guo
guofenglinux@gmail.com

While fixing the assertion failure with LATERAL UNION ALL reported by
Fujii-san [1]/messages/by-id/CAHGQGwGZBZmb7Yq+UTT0Lp5wvztQZtMT94ehcMHQs9Dk6jaaXQ@mail.gmail.com, I ran into this:

create table t (a int);

select *
from t t1 left join (t t2 join t t3 on true) j on true,
lateral ((select j offset 0) union all (select null offset 0)) ss;
ERROR: too late to create a new PlaceHolderInfo

This goes back to v16, when add_nullingrels_if_needed() was first
introduced.

When a LATERAL UNION ALL subquery is pulled up as an appendrel,
find_lateral_references looks only at the parent RTE's subquery,
assuming the children are copies of parts of it. But the parent's
subquery and the children get their join alias Vars expanded
separately, later in subquery_planner. If the expansion needs a PHV
(here, the whole-row Var of j, which is nulled by the left join), each
flatten_join_alias_vars call makes its own PHV with a new phid. So
the children end up asking for PHVs that have no PlaceHolderInfo.

A straightforward fix is to expand join alias Vars in the LATERAL
subquery in pull_up_simple_union_all, before its rtable gets copied,
so that all the copies share the same PHVs. Doing this that early
should be OK, since a LATERAL item can only reference FROM items to
its left, and pull_up_subqueries has already processed those, so their
join alias lists are final. See attached.

This feels more like a band-aid though. I think the more principled
fix would be a PHV cache in add_nullingrels_if_needed, so that we
don't generate identical PHVs with different IDs, like what rv_cache
does in pullup_replace_vars_callback. But since
flatten_join_alias_vars is called many times per query level, the
cache would have to live in PlannerInfo, which seems too invasive to
back-patch.

Any thoughts?

[1]: /messages/by-id/CAHGQGwGZBZmb7Yq+UTT0Lp5wvztQZtMT94ehcMHQs9Dk6jaaXQ@mail.gmail.com

- Richard

Attachments:

t253816_1
v1-0001-Fix-mismatched-PHVs-for-join-aliases-in-LATERAL-U.patchapplication/octet-stream; name=v1-0001-Fix-mismatched-PHVs-for-join-aliases-in-LATERAL-U.patchDownload+164-1
#2Richard Guo
guofenglinux@gmail.com
In reply to: Richard Guo (#1)
Re: ERROR: too late to create a new PlaceHolderInfo

On Thu, Sep 17, 2026 at 10:14 AM Richard Guo <guofenglinux@gmail.com> wrote:

A straightforward fix is to expand join alias Vars in the LATERAL
subquery in pull_up_simple_union_all, before its rtable gets copied,
so that all the copies share the same PHVs.

Ugh, this doesn't work if the UNION ALL is nested inside another
LATERAL subquery:

select *
from t t1 left join (t t2 join t t3 on true) j on true,
lateral (select * from ((select j offset 0) union all (select null
offset 0)) s) ss;
ERROR: too late to create a new PlaceHolderInfo

Here the UNION ALL gets pulled up while ss is being pulled up, using
ss's own subroot. At that point its RTE isn't LATERAL, and j belongs
to the outer query anyway, so v1 doesn't expand anything. We could
patch pull_up_simple_subquery the same way, but that makes the fix
more like a band-aid.

This feels more like a band-aid though. I think the more principled
fix would be a PHV cache in add_nullingrels_if_needed, so that we
don't generate identical PHVs with different IDs, like what rv_cache
does in pullup_replace_vars_callback. But since
flatten_join_alias_vars is called many times per query level, the
cache would have to live in PlannerInfo, which seems too invasive to
back-patch.

I still think the PHV cache is the better approach. Attached v2 does
that: add_nullingrels_if_needed now keeps the PHVs it makes in a new
PlannerInfo field, and reuses one when the same expression is expanded
again. This fixes both queries, no matter where the copies come from,
and it also avoids generating duplicate PHVs for the same alias
elsewhere in the query.

The problem with it is that it breaks ABI and can't be back-patched.
Given that there have been no field reports since v16, maybe we can
fix it on master only?

- Richard

Attachments:

t253816_2
v2-0001-Fix-mismatched-PHVs-for-join-aliases-in-LATERAL-U.patchapplication/octet-stream; name=v2-0001-Fix-mismatched-PHVs-for-join-aliases-in-LATERAL-U.patchDownload+250-4
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Richard Guo (#2)
Re: ERROR: too late to create a new PlaceHolderInfo

Richard Guo <guofenglinux@gmail.com> writes:

This feels more like a band-aid though. I think the more principled
fix would be a PHV cache in add_nullingrels_if_needed, so that we
don't generate identical PHVs with different IDs, like what rv_cache
does in pullup_replace_vars_callback. But since
flatten_join_alias_vars is called many times per query level, the
cache would have to live in PlannerInfo, which seems too invasive to
back-patch.

I agree with that sounding more principled, but I wonder if we should
think bigger than just tweaking add_nullingrels_if_needed: if we're
desirous of de-duplicating PHVs, why not do that across the board,
for every place that makes PHVs? So we'd mechanize this in
make_placeholder_expr's assignment of phid rather than somewhere else.

The problem with it is that it breaks ABI and can't be back-patched.
Given that there have been no field reports since v16, maybe we can
fix it on master only?

Maybe. In the past, when we needed a new struct field, we've sometimes
decided that adding it at the end of the struct in the back branches
would be adequately ABI-compatible. I think we might be able to get
away with that here too, although I'm a bit worried whether any
extensions might be doing makeNode(PlannerInfo).

regards, tom lane

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#3)
Re: ERROR: too late to create a new PlaceHolderInfo

I wrote:

I agree with that sounding more principled, but I wonder if we should
think bigger than just tweaking add_nullingrels_if_needed: if we're
desirous of de-duplicating PHVs, why not do that across the board,
for every place that makes PHVs? So we'd mechanize this in
make_placeholder_expr's assignment of phid rather than somewhere else.

Actually, we can't be too gung-ho about that: we should not merge
PHVs if their expressions are volatile. It's not quite clear to me
whether that's a problem for the join-alias-Vars case.

regards, tom lane