Re: EphemeralNamedRelation and materialized view

On Mon, 30 Dec 2024 16:06:06 -0500
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp> writes:
> > On Wed, 20 Nov 2024 12:43:16 -0500
> > Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> ... It seems to me that we should
> >> think about this, for MVs as well as those other object types,
> >> as fundamentally a dependency problem. That is, the reason
> >> we can't allow a reference to an ENR in a long-lived object
> >> is that we have no catalog representation for the reference.
> >> So that leads to thinking that the issue ought to be handled
> >> in recordDependencyOnExpr and friends. If we see an ENR while
> >> scanning a rangetable to extract dependencies, then complain.
> >> This might be a bit messy to produce good error messages for,
> >> though.
>
> > I've attached a updated patch. Use of ENRs are now checked in
> > find_expr_references_walker() called from recordDependencyOnExpr().
>
> This looks pretty good to me, except that I question the use of
> getObjectTypeDescription() in the error message. There are a
> few things not to like about that:
>
> 1. This is kind of an off-label use of getObjectTypeDescription,
> in that we can't expect the object to be visible yet in the catalogs.
> Yeah, we can hack around that by passing missing_ok = true, but it
> still seems like a kluge.
>
> 2. The grammar isn't great, and translatability of the message
> would be poor I think.
>
> 3. As your test case demonstrates, the message is going to complain
> about a "rule" if the problem is with a view or matview, because
> we represent the dependency as being from the view's ON SELECT rule.
> This seems quite confusing for anyone not deeply versed in PG's inner
> workings.
>
> After some thought I propose that we just complain that a "persistent
> object" can't depend on a transition table, and not try to identify
> the depender any more closely than that. We can still add some
> context to the message by showing the transition table's name,
> since that's readily available from the RTE. See attached v3,
> where I also did a bit of editing of the comments and test case.

Thank you for your reviewing and editing the patch!
I agree with your proposal on the error message handling.

> BTW, I'm not entirely convinced that the first addition (in Var
> processing) is necessary. Such a Var must refer to an RTE
> somewhere, and I'm having a hard time coming up with a case
> where the RTE wouldn't also be part of what we scan for
> dependencies. It's harmless enough to have the extra check,
> but can you think of a case where it's actually needed?

On second thought, I could not think of such a case. This part
can be removed. I attached v4 patch.

Regards,
Yugo Nagata

--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>