Re: EphemeralNamedRelation and materialized view

On Mon, 30 Dec 2024 16:06:06 -0500
Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Yugo NAGATA <nagata@sraoss.co.jp> writes:
> > On Wed, 20 Nov 2024 12:43:16 -0500
> > Tom Lane <tgl@sss.pgh.pa.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@sraoss.co.jp>