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