Re: Schema variables - new implementation for Postgres 15

pá 31. 3. 2023 v 21:31 odesílatel Dmitry Dolgov <9erthalion6@gmail.com>
napsal:

> > On Tue, Mar 28, 2023 at 09:34:20PM +0200, Pavel Stehule wrote:
> > Hi
> >
> > > Talking about documentation I've noticed that the implementation
> > > contains few limitations, that are not mentioned in the docs. Examples
> > > are WITH queries:
> > >
> > >     WITH x AS (LET public.svar = 100) SELECT * FROM x;
> > >     ERROR:  LET not supported in WITH query
> > >
> >
> >  The LET statement doesn't support the RETURNING clause, so using inside
> > CTE does not make any sense.
> >
> > Do you have some tips, where this behaviour should be mentioned?
>
> Yeah, you're right, it's probably not worth adding. I usually find it a
> good idea to explicitly mention any limitations, but WITH docs are
> actually have one line about statements without the RETURNING clause,
> plus indeed for LET it makes even less sense.
>
> > > and using with set-returning functions (haven't found any related
> tests).
> > >
> >
> > There it is:
> >
> > +CREATE VARIABLE public.svar AS int;
> > +-- should be ok
> > +LET public.svar = generate_series(1, 1);
> > +-- should fail
> > +LET public.svar = generate_series(1, 2);
> > +ERROR:  expression returned more than one row
> > +LET public.svar = generate_series(1, 0);
> > +ERROR:  expression returned no rows
> > +DROP VARIABLE public.svar;
>
> Oh, interesting. I was looking for another error message from
> parse_func.c:
>
>     set-returning functions are not allowed in LET assignment expression
>
> Is this one you've posted somehow different?
>

This limit is correct, but the error message is maybe messy - I changed it.

This is protection against:

(2023-04-01 06:25:50) postgres=# create variable xxx as int[];
CREATE VARIABLE
(2023-04-01 06:26:02) postgres=# let xxx[generate_series(1,3)] = 10;
ERROR:  set-returning functions are not allowed in LET assignment expression
LINE 1: let xxx[generate_series(1,3)] = 10;
                ^

change:
        case EXPR_KIND_LET_TARGET:
-           err = _("set-returning functions are not allowed in LET
assignment expression");
+           err = _("set-returning functions are not allowed in LET target
expression");
            break;

This case was not tested - so I did new test for this case


> > > Another small note is about this change in the rowsecurity:
> > >
> > >         /*
> > >     -    * For SELECT, UPDATE and DELETE, add security quals to enforce
> > > the USING
> > >     -    * policies.  These security quals control access to existing
> > > table rows.
> > >     -    * Restrictive policies are combined together using AND, and
> > > permissive
> > >     -    * policies are combined together using OR.
> > >     +    * For SELECT, LET, UPDATE and DELETE, add security quals to
> > > enforce the
> > >     +    * USING policies.  These security quals control access to
> > > existing table
> > >     +    * rows. Restrictive policies are combined together using AND,
> and
> > >     +    * permissive policies are combined together using OR.
> > >          */
> > >
> > > From this commentary one may think that LET command supports row level
> > > security, but I don't see it being implemented. A wrong commentary?
> > >
> >
> > I don't think so.  The row level security should be supported. I tested
> it
> > on example from doc:
> >
> > [...]
> >
> > (2023-03-28 21:32:33) postgres=# set role to t1role;
> > SET
> > (2023-03-28 21:32:40) postgres=# select * from accounts ;
> > ┌─────────┬─────────┬────────────────┐
> > │ manager │ company │ contact_email  │
> > ╞═════════╪═════════╪════════════════╡
> > │ t1role  │ xxx     │ t1role@xxx.org │
> > └─────────┴─────────┴────────────────┘
> > (1 row)
> >
> > (2023-03-28 21:32:45) postgres=# let v = (select company from accounts);
> > LET
> > (2023-03-28 21:32:58) postgres=# select v;
> > ┌─────┐
> > │  v  │
> > ╞═════╡
> > │ xxx │
> > └─────┘
> > (1 row)
> >
> > (2023-03-28 21:33:03) postgres=# set role to default;
> > SET
> > (2023-03-28 21:33:12) postgres=# set role to t2role;
> > SET
> > (2023-03-28 21:33:19) postgres=# select * from accounts ;
> > ┌─────────┬─────────┬────────────────┐
> > │ manager │ company │ contact_email  │
> > ╞═════════╪═════════╪════════════════╡
> > │ t2role  │ yyy     │ t2role@yyy.org │
> > └─────────┴─────────┴────────────────┘
> > (1 row)
> >
> > (2023-03-28 21:33:22) postgres=# let v = (select company from accounts);
> > LET
> > (2023-03-28 21:33:26) postgres=# select v;
> > ┌─────┐
> > │  v  │
> > ╞═════╡
> > │ yyy │
> > └─────┘
> > (1 row)
>
> Hm, but isn't the row level security enforced here on the select level,
> not when assigning some value via LET? Plus, it seems the comment
> originally refer to the command types (CMD_SELECT, etc), and there is no
> CMD_LET and no need for it, right?
>
> I'm just trying to understand if there was anything special done for
> session variables in this regard, and if not, the commentary change
> seems to be not needed (I know, I know, it's totally nitpicking).
>

I am not sure at this point.  It is true, so it doesn't modify any lines
there, and this is the reason why this comment is maybe messy.

I'll remove it.

p.s. I am sending an updated patch still in the old format. Refactoring to
a new format for Peter can take some time, and the patch in the old format
can be available for people who can do some tests or some checks.



Regards

Pavel