Re: Schema variables - new implementation for Postgres 15

From: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Sergey Shinderuk <s(dot)shinderuk(at)postgrespro(dot)ru>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, dean(dot)a(dot)rasheed(at)gmail(dot)com, er(at)xs4all(dot)nl, joel(at)compiler(dot)org, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Schema variables - new implementation for Postgres 15
Date: 2023-03-03 20:17:57
Message-ID: 20230303201757.gvc5txqbvlcutaqc@erthalion.local
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Tue, Feb 28, 2023 at 06:12:50AM +0100, Pavel Stehule wrote:
>
> fresh rebase

I'm continuing to review, this time going through shadowing stuff in
transformColumnRef, IdentifyVariable etc. Well, that's a lot of leg work
for rather little outcome :) I guess all attempts to simplify this part
weren't successful?

Couple of questions to it. In IdentifyVariable in the branch handling
two values the commentary says:

/*
* a.b can mean "schema"."variable" or "variable"."field",
* Check both variants, and returns InvalidOid with
* not_unique flag, when both interpretations are
* possible. Second node can be star. In this case, the
* only allowed possibility is "variable"."*".
*/

I read this as "variable"."*" is a valid combination, but the very next
part of this condition says differently:

/*
* Session variables doesn't support unboxing by star
* syntax. But this syntax have to be calculated here,
* because can come from non session variables related
* expressions.
*/
Assert(IsA(field2, A_Star));

Is the first commentary not quite correct?

Another question about how shadowing warning should work between namespaces.
Let's say I've got two namespaces, public and test, both have a session
variable with the same name, but only one has a table with the same name:

-- in public
create table test_agg(a int);
create type for_test_agg as (a int);
create variable test_agg for_test_agg;

-- in test
create type for_test_agg as (a int);
create variable test_agg for_test_agg;

Now if we will try to trigger the shadowing warning from public
namespace, it would work differently:

-- in public
=# let test.test_agg.a = 10;
=# let test_agg.a = 20;
=# set session_variables_ambiguity_warning to on;

-- note the value returned from the table
=# select jsonb_agg(test_agg.a) from test_agg;
WARNING: 42702: session variable "test_agg.a" is shadowed
LINE 1: select jsonb_agg(test_agg.a) from test_agg;
^
DETAIL: Session variables can be shadowed by columns, routine's variables and routine's arguments with the same name.
LOCATION: transformColumnRef, parse_expr.c:940
jsonb_agg
-----------
[1]

-- no warning, note the session variable value
=# select jsonb_agg(test.test_agg.a) from test_agg;
jsonb_agg
-----------
[10]

It happens because in the second scenario the logic inside transformColumnRef
will not set up the node variable (there is no corresponding table in the
"test" schema), and the following conditions covering session variables
shadowing are depending on it. Is it supposed to be like this?

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2023-03-03 21:38:05 Re: zstd compression for pg_dump
Previous Message Dean Rasheed 2023-03-03 20:16:15 Re: SQL JSON path enhanced numeric literals