Re: Schema variables - new implementation for Postgres 15

Pavel Stehule <pavel.stehule@gmail.com>

From: Pavel Stehule <pavel.stehule@gmail.com>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>, Joel Jacobson <joel@compiler.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-01-30T13:15:31Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Allow underscores in integer and numeric constants.

  2. Remove special outfuncs/readfuncs handling of RangeVar.catalogname.

  3. Remove extra space from dumped ALTER DEFAULT PRIVILEGES.

  4. Create FKs properly when attaching table as partition

  5. psql: improve tab-complete's handling of variant SQL names.

Attachments

so 29. 1. 2022 v 6:19 odesílatel Julien Rouhaud <rjuju123@gmail.com> napsal:

> Hi,
>
> On Fri, Jan 28, 2022 at 07:51:08AM +0100, Pavel Stehule wrote:
> > st 26. 1. 2022 v 8:23 odesílatel Julien Rouhaud <rjuju123@gmail.com>
> napsal:
> >
> > > +      The <literal>ON TRANSACTION END RESET</literal>
> > > +      clause causes the session variable to be reset to its default
> value
> > > when
> > > +      the transaction is committed or rolled back.
> > >
> > > As far as I can see this clauses doesn't play well with IMMUTABLE
> > > VARIABLE, as
> > > you can reassign a value once the transaction ends.  Same for DISCARD [
> > > ALL |
> > > VARIABLES ], or LET var = NULL (or DEFAULT if no default value).  Is
> that
> > > intended?
> > >
> >
> > I think so it is expected. The life scope of assigned (immutable) value
> is
> > limited to transaction (when ON TRANSACTION END RESET).
> > DISCARD is used for reset of session, and after it, you can write the
> value
> > first time.
> >
> > I enhanced doc in IMMUTABLE clause
>
> I think it's still somewhat unclear:
>
> -      done, no other change will be allowed in the session lifetime.
> +      done, no other change will be allowed in the session variable
> content's
> +      lifetime. The lifetime of content of session variable can be
> +      controlled by <literal>ON TRANSACTION END RESET</literal> clause.
> +     </para>
>
> The "session variable content lifetime" is quite peculiar, as the ON
> TRANSACTION END RESET is adding transactional behavior to something that's
> not
> supposed to be transactional, so more documentation about it seems
> appropriate.
>
> Also DISCARD can be used any time so that's a totally different aspect of
> the
> immutable variable content lifetime that's not described here.
>

fixed




>
> NULL handling also seems inconsistent.  An explicit default NULL value
> makes it
> truly immutable, but manually assigning NULL is a different codepath that
> has a
> different user behavior:
>
> # create immutable variable var_immu int default null;
> CREATE VARIABLE
>
> # let var_immu = 1;
> ERROR:  22005: session variable "var_immu" is declared IMMUTABLE
>
> # create immutable variable var_immu2 int ;
> CREATE VARIABLE
>
> # let var_immu2 = null;
> LET
>
> # let var_immu2 = null;
> LET
>
> # let var_immu2 = 1;
> LET
>
> For var_immu2 I think that the last 2 queries should have errored out.
>

ok, I changed this behave


>
> > > In revoke.sgml:
> > > + REVOKE [ GRANT OPTION FOR ]
> > > +     { { READ | WRITE } [, ...] | ALL [ PRIVILEGES ] }
> > > +     ON VARIABLE <replaceable>variable_name</replaceable> [, ...]
> > > +     FROM { [ GROUP ] <replaceable
> > > class="parameter">role_name</replaceable> | PUBLIC } [, ...]
> > > +     [ CASCADE | RESTRICT ]
> > >
> > > there's no extra documentation for that, and therefore no
> clarification on
> > > variable_name.
> > >
> >
> > This is same like function_name, domain_name, ...
>
> Ah right.
>
> > > pg_variable.c:
> > > Do we really need both session_variable_get_name() and
> > > get_session_variable_name()?
> > >
> >
> > They are different - first returns possibly qualified name, second
> returns
> > only name. Currently it is used just for error messages in
> > transformAssignmentIndirection, and I think so it is good for consistency
> > with other usage of this routine (transformAssignmentIndirection).
>
> I agree that consistency with other usage is a good thing, but both
> functions
> have very similar and confusing names.  Usually when you need the qualified
> name the calling code just takes care of doing so.  Wouldn't it be better
> to
> add say get_session_variable_namespace() and construct the target string
> in the
> calling code?
>

ok, I rewrote related code


>
> Also, I didn't dig a lot but I didn't see other usage with optionally
> qualified
> name there?  I'm not sure how it would make sense anyway since LET
> semantics
> are different and the current call for session variable emit incorrect
> messages:
>

changed


> # create table tt(id integer);
> CREATE TABLE
>
> # create variable vv tt;
> CREATE VARIABLE
>
> # let vv.meh = 1;
> ERROR:  42703: cannot assign to field "meh" of column "meh" because there
> is no such column in data type tt
> LINE 1: let vv.meh = 1;
>

 fixed

postgres=#  create table tt(id integer); create variable vv tt;
CREATE TABLE
CREATE VARIABLE
postgres=# let vv.meh = 1;
ERROR:  cannot assign to field "meh" of column or variable "vv" because
there is no such column in data type tt
LINE 1: let vv.meh = 1;
            ^

Regards

Pavel