Re: Schema variables - new implementation for Postgres 15

> On Wed, May 22, 2024 at 08:44:28PM +0200, Pavel Stehule wrote:
> st 22. 5. 2024 v 19:25 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:
>
> > Peter Eisentraut <peter(at)eisentraut(dot)org> writes:
> > > On 18.05.24 13:29, Alvaro Herrera wrote:
> > >> I want to note that when we discussed this patch series at the dev
> > >> meeting in FOSDEM, a sort-of conclusion was reached that we didn't want
> > >> schema variables at all because of the fact that creating a variable
> > >> would potentially change the meaning of queries by shadowing table
> > >> columns. But this turns out to be incorrect: it's_variables_ that are
> > >> shadowed by table columns, not the other way around.
> >
> > > But that's still bad, because seemingly unrelated schema changes can
> > > make variables appear and disappear. For example, if you have
> > > SELECT a, b FROM table1
> > > and then you drop column b, maybe the above query continues to work
> > > because there is also a variable b.
> >
> > Yeah, that seems pretty dangerous. Could we make it safe enough
> > by requiring some qualification on variable names? That is, if
> > you mean b to be a variable, then you must write something like
> >
> > SELECT a, pg_variables.b FROM table1
> >
> > This is still ambiguous if you use "pg_variables" as a table alias in
> > the query, but the alias would win so the query still means what it
> > meant before. Also, table aliases (as opposed to actual table names)
> > don't change readily, so I don't think there's much risk of the query
> > suddenly meaning something different than it did yesterday.
> >
>
> With active shadowing variable warning for described example you will get a
> warning before dropping.

I assume you're talking about a warning, which one will get querying the
table with shadowed columns. If no such query has happened yet and the
column was dropped, there will be no warning.

Aside that, I'm afraid dropping a warning in log does not have
sufficient visibility to warn about the issue, since one needs to read
those logs first. I guess what folks are looking for is more constraints
out of the box, preventing any ambiguity.

> Session variables are joined with schema (in my proposal). Do anybody can
> do just
>
> CREATE SCHEMA svars; -- or what (s)he likes
> CREATE VARIABLE svars.b AS int;
>
> SELECT a, b FROM table1
>
> and if somebody can be really safe, the can write
>
> SELECT t.a, t.b FROM table1 t
>
> or
>
> SELECT t.a, svars.b FROM table1 t
>
> It can be customized in the way anybody prefers - just creating dedicated
> schemas and setting search_path. Using its own schema for session variables
> without enhancing search_path for this schema forces the necessity to set
> only qualified names for session variables.
>
> Sure the naming of schemas, aliases can be unhappy wrong, and there can be
> the problem. But this can be a problem today too.

If I understand you correctly, you're saying that there are "best
practices" how to deal with session variables to avoid any potential
issues. But I think it's more user-friendly to have something that will
not allow shooting yourself in the foot right out of the box. You're
right, similar things could probably happen with the already existing
functionality, but it doesn't give us rights to add more to it.
Especially if it's going to be about a brand-new feature.

As far as I can see now, it's a major design flaw that could keep the
patch from being accepted. Fortunately there are few good proposals how
to address this, folks are genuinely trying to help. What do you think
about trying some of them out, as an alternative approach, to compare
functionality and user experience?

In the meantime I'm afraid I have to withdraw "Ready for committer"
status, sorry. I've clearly underestimated the importance of variables
shadowing, thanks Alvaro and Peter for pointing out some dangerous
cases. I still believe though that the majority of the patch is in a
good shape and the question about variables shadowing is the only thing
that keeps it from moving forward.