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
