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.