Re: Schema variables - new implementation for Postgres 15

From: Julien Rouhaud <rjuju123(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Joel Jacobson <joel(at)compiler(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Schema variables - new implementation for Postgres 15
Date: 2022-01-19 22:03:40
Message-ID: 20220119220340.m5xtlwuinywcncvd@jrouhaud
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Wed, Jan 19, 2022 at 09:09:41PM +0100, Pavel Stehule wrote:
> st 19. 1. 2022 v 9:01 odesílatel Julien Rouhaud <rjuju123(at)gmail(dot)com> napsal:
>
> RAISE NOTICE should use local variables, and in this case is a question if we
> should raise a warning. There are two possible analogies - we can see session
> variables like global variables, and then the warning should not be raised,
> or we can see relation between session variables and plpgsql variables
> similar like session variables and some with higher priority, and then
> warning should be raised. If we want to ensure that the new session variable
> doesn't break code, then session variables should have lower priority than
> plpgsql variables too. And because the plpgsql protection against collision
> cannot be used, then I prefer raising the warning.

Ah that's indeed a good point. I agree, they're from a different part of the
system so they should be treated as different things, and thus raising a
warning. It's consistent with the chosen conservative approach anyway.

> PLpgSQL assignment should not be in collision with session variables ever

Agreed.

>
> >
> > =# DO $$
> > BEGIN
> > v := 'test';
> > RAISE NOTICE 'v: %', v;
> > END;
> > $$ LANGUAGE plpgsql;
> > ERROR: 42601: "v" is not a known variable
> > LINE 3: v := 'test';
> >
> > But the RAISE NOTICE does see the session variable (which should be the
> > correct
> > behavior I think), so the warning should have been raised for this
> > instruction
> > (and in that case the message is incorrect, as it's not shadowing a
> > column).
> >
>
> In this case I can detect node type, and I can identify external param
> node, but I cannot to detect if this code was executed from PLpgSQL or from
> some other
>
> So I can to modify warning text to some

Yes, that's what I had in mind too.

> DETAIL: The identifier can be column reference or query parameter or
> session variable reference.
> HINT: The column reference and query parameter is preferred against
> session variable reference.
>
> I cannot to use term "plpgsql variable" becase I cannot to ensure validity
> of this message
>
> Maybe is better to don't talk about source of this issue, and just talk
> about result - so the warning text should be just
>
> MESSAGE: "session variable \"xxxx\" is shadowed
> DETAIL: "session variables can be shadowed by columns, routine's variables
> and routine's arguments with same name"
>
> Is it better?

I clearly prefer the 2nd version.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhihong Yu 2022-01-19 22:13:34 Re: missing indexes in indexlist with partitioned tables
Previous Message Arne Roland 2022-01-19 21:50:01 Re: missing indexes in indexlist with partitioned tables