Bogus rte->relkind for EXCLUDED pseudo-relation
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:t47040psql -h localhost -U postgresBuilt from patchset v1 (message #1), September 20, 2026 at 11:08 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 t47040_1 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 t47040_1 && git checkout t47040_1Patchset v1 (message #1) is on t47040_1
In the wake of b23cd185f (pushed just now), I tried adding Asserts
to rewriteHandler.c that relkinds in RTEs don't change, as attached.
This blew up the regression tests immediately. On investigation,
I find that
(1) ON CONFLICT's EXCLUDED pseudo-relation is assigned
rte->relkind = RELKIND_COMPOSITE, a rather horrid hack
installed by commit ad2278379.
(2) If a stored rule involves ON CONFLICT, then while loading
the rule AcquireRewriteLocks overwrites that with the actual
relkind, ie RELKIND_RELATION. Or it did without the
attached Assert, anyway.
It appears to me that this means whatever safeguards are created
by the use of RELKIND_COMPOSITE will fail to apply in a rule.
Maybe that's okay because the relevant behaviors only occur at
parse time not rewrite/planning/execution, but even to write that
is to doubt how reliable and future-proof the assumption is.
I'm inclined to think we'd be well advised to undo that aspect of
ad2278379 and solve it some other way. Maybe a new RTEKind would
be a better idea. Alternatively, we could drop rewriteHandler.c's
attempts to update relkind. Theoretically that's safe now, but
I hadn't quite wanted to just pull that trigger right away ...
regards, tom lane
Hi,
On 2022-12-02 12:34:36 -0500, Tom Lane wrote:
In the wake of b23cd185f (pushed just now), I tried adding Asserts
to rewriteHandler.c that relkinds in RTEs don't change, as attached.
This blew up the regression tests immediately. On investigation,
I find that(1) ON CONFLICT's EXCLUDED pseudo-relation is assigned
rte->relkind = RELKIND_COMPOSITE, a rather horrid hack
installed by commit ad2278379.
Is it that horrid? I guess we can add a full blown relkind for it, but that'd
not really change that we'd logic to force AcquireRewriteLocks() to keep it's
hand off the relkind?
We don't really have a different way to represent something that looks like a
table's tuple, but without system columns, and that shouldn't affected by RLS
rewrite magic. We could add a distinct RELKIND of course, but that'd afaict
look very similar to RELKIND_COMPOSITE_TYPE.
I'm inclined to think we'd be well advised to undo that aspect of
ad2278379 and solve it some other way. Maybe a new RTEKind would
be a better idea. Alternatively, we could drop rewriteHandler.c's
attempts to update relkind. Theoretically that's safe now, but
I hadn't quite wanted to just pull that trigger right away ...
I think it'd be good to not have rewriteHandler.c update relkind, even if we
undo the RELKIND_COMPOSITE aspect of ad2278379. Changing relkind seems fairly
dangerous to me, particularly because we don't ever expect that to happen
now. I think it might make sense to make it an elog() rather than an Assert()
though.
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
On 2022-12-02 12:34:36 -0500, Tom Lane wrote:
I find that
(1) ON CONFLICT's EXCLUDED pseudo-relation is assigned
rte->relkind = RELKIND_COMPOSITE, a rather horrid hack
installed by commit ad2278379.
Is it that horrid?
It's pretty bad IMO. You didn't even bother to update the comments
for RangeTblEntry to explain that
char relkind; /* relation kind (see pg_class.relkind) */
might now not be the rel's relkind at all. Changing RTEKind
would likely be a better way, though of course we couldn't
do that in back branches.
And I think that we do have an issue in the back branches.
According to the commit message for ad2278379,
4) References to EXCLUDED were rewritten by the RLS machinery, as
EXCLUDED was treated as if it were the underlying relation.
That rewriting would be post-rule-load, so it sure seems to me
that a rule containing EXCLUDED would be improperly subject to
RLS rewriting. I don't have enough familiarity with RLS to come
up with a test case, and I don't see any relevant examples in
the mailing list threads referenced by ad2278379, but I bet
that it is broken.
The back-branch fix could just be to teach rewriteHandler.c
to not overwrite RELKIND_COMPOSITE_TYPE, perhaps. We can't
remove the update completely because of the table-to-view case.
regards, tom lane