From: Julien Rouhaud <rjuju123@gmail.com>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Joel Jacobson <joel@compiler.org>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Schema variables - new implementation for Postgres 15
Date: Thu, 20 Jan 2022 06:03:40 +0800
Message-ID: <20220119220340.m5xtlwuinywcncvd@jrouhaud> (raw)
In-Reply-To: <CAFj8pRC24VHS0YujMr42T12o15gcABhpuarhDku7_nngtsxUnA@mail.gmail.com>
References: <b846763e-1f9a-40a8-ac67-a3a4585826ef@www.fastmail.com>
<CAFj8pRBT-bRQJBqkzon7tHcoFZ1byng06peZfZa0G72z46YFxg@mail.gmail.com>
<1ebeaf57-da83-42e1-9302-fb126281814d@www.fastmail.com>
<CAFj8pRD9iEpPN=qX1FKtNxYha9B48sZVqAjFgck7Ah0DpFckOg@mail.gmail.com>
<CAEZATCUdkmVBKCy8WcLinMP9rjFD8hHrZ0Qy4tMEor_P_GHv2g@mail.gmail.com>
<CAFj8pRDT4RW_ZiAbsbRGjTDZ+7on3y09dV04KVAEKya_HTo=1A@mail.gmail.com>
<20220114024402.btyn7vrpkhnso2bq@jrouhaud>
<CAFj8pRC9dWjbiP2d4naacU4FYgE9-NSTwGtq+M71GnaYL+odog@mail.gmail.com>
<20220119080117.4c2fjzs5p7vtdxhk@jrouhaud>
<CAFj8pRC24VHS0YujMr42T12o15gcABhpuarhDku7_nngtsxUnA@mail.gmail.com>
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@gmail.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.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: rjuju123@gmail.com, pavel.stehule@gmail.com, dean.a.rasheed@gmail.com, joel@compiler.org, pgsql-hackers@lists.postgresql.org
Subject: Re: Schema variables - new implementation for Postgres 15
In-Reply-To: <20220119220340.m5xtlwuinywcncvd@jrouhaud>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox