> 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
