Re: proposal: schema variables

pá 19. 7. 2024 v 13:41 odesílatel Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
napsal:

> On Sat, 2021-04-10 at 08:58 +0200, Pavel Stehule wrote:
> > I am sending a strongly updated patch for schema variables.
> >
> > I rewrote an execution of a LET statement. In the previous
> implementation I hacked
> > STMT_SELECT. Now, I introduced a new statement STMT_LET, and I
> implemented a new
> > executor node SetVariable. Now I think this implementation is much
> cleaner.
> > Implementation with own executor node reduces necessary work on PL side
> - and allows
> > the LET statement to be prepared - what is important from a security
> view.
> >
> > I'll try to write a second implementation based on a cleaner
> implementation like
> > utility command too. I expect so this version will be more simple, but
> utility
> > commands cannot be prepared, and probably, there should be special
> support for
> > any PL. I hope a cleaner implementation can help to move this patch.
> >
> > We can choose one variant in the next step and this variant can be
> finalized.
> >
> > Notes, comments?
>
> Thank you!
>
> I tried to give the patch a spin, but it doesn't apply any more,
> and there are too many conflicts for me to fix manually.
>
> So I had a look at the documentation:
>
> > --- a/doc/src/sgml/advanced.sgml
> > +++ b/doc/src/sgml/advanced.sgml
>
> > + <para>
> > + The value of a schema variable is local to the current session.
> Retrieving
> > + a variable's value returns either a NULL or a default value, unless
> its value
> > + is set to something else in the current session with a LET command.
> The content
> > + of a variable is not transactional. This is the same as in regular
> variables
> > + in PL languages.
> > + </para>
> > +
> > + <para>
> > + Schema variables are retrieved by the <command>SELECT</command> SQL
> command.
> > + Their value is set with the <command>LET</command> SQL command.
> > + While schema variables share properties with tables, their value
> cannot be updated
> > + with an <command>UPDATE</command> command.
>
> "PL languages" -> "procedural languages". Perhaps a link to the
> "procedural Languages"
> chapter would be a good idea.
> I don't think we should say "regular" variables: are there irregular
> variables?
>
> My feeling is that "SQL statement <command>XY</command>" is better than
> "<command>XY</command> SQL command".
>

probably, you are reading an old version of this patch. I cannot find these
sentences.

>
> I think the last sentence should go. The properties they share with
> tables are
> that they live in a schema and can be used with SELECT.
> Also, it is not necessary to mention that they cannot be UPDATEd. They
> cannot
> be TRUNCATEd or CALLed either, so why mention UPDATE specifically?
>
> > --- a/doc/src/sgml/catalogs.sgml
> > +++ b/doc/src/sgml/catalogs.sgml
>
> > + <row>
> > + <entry><structfield>varisnotnull</structfield></entry>
> > + <entry><type>boolean</type></entry>
> > + <entry></entry>
> > + <entry>
> > + True if the schema variable doesn't allow null value. The
> default value is false.
> > + </entry>
> > + </row>
>
> I think the attribute should be called "varnotnull", similar to
> "attnotnull".
> This attribute determines whether the variable is NOT NULL or not, not
> about
> its current setting.
>
> There is a plural missing: "doesn't allow null valueS".
>

changed

>
> > + <row>
> > + <entry><structfield>vareoxaction</structfield></entry>
> > + <entry><type>char</type></entry>
> > + <entry></entry>
> > + <entry>
> > + <literal>n</literal> = no action, <literal>d</literal> = drop
> the variable,
> > + <literal>r</literal> = reset the variable to its default value.
> > + </entry>
> > + </row>
>
> Perhaps the name "varxactendaction" would be better.
>
> A descriptive sentence is missing.
>

I renamed field, recent version looks like

<row>
<entry role="catalog_table_entry"><para role="column_definition">
<structfield>varxactendaction</structfield> <type>char</type>
</para>
<para>
Action performed at end of transaction:
<literal>n</literal> = no action, <literal>d</literal> = drop the
variable,
<literal>r</literal> = reset the variable to its default value.
</para></entry>
</row>

>
> > --- /dev/null
> > +++ b/doc/src/sgml/ref/create_variable.sgml
>
> > + <para>
> > + The value of a schema variable is local to the current session.
> Retrieving
> > + a variable's value returns either a NULL or a default value, unless
> its value
> > + is set to something else in the current session with a LET command.
> The content
> > + of a variable is not transactional. This is the same as in regular
> variables in PL languages.
> > + </para>
>
> "regular variables in PL languages" -> "variables in procedural languages"
>

fixed

>
> > + <para>
> > + Schema variables are retrieved by the <command>SELECT</command> SQL
> command.
> > + Their value is set with the <command>LET</command> SQL command.
> > + While schema variables share properties with tables, their value
> cannot be updated
> > + with an <command>UPDATE</command> command.
> > + </para>
>
> That's just a literal copy from the tutorial section. I have the same
> comments
> as there.
>

fixed

>
> > + <varlistentry>
> > + <term><literal>NOT NULL</literal></term>
> > + <listitem>
> > + <para>
> > + The <literal>NOT NULL</literal> clause forbids to set the
> variable to
> > + a null value. A variable created as NOT NULL and without an
> explicitly
> > + declared default value cannot be read until it is initialized by
> a LET
> > + command. This obliges the user to explicitly initialize the
> variable
> > + content before reading it.
> > + </para>
> > + </listitem>
> > + </varlistentry>
>
> What is the reason for that behavior? I'd have expected that a NOT NULL
> variable needs a default value.
>

changed - now, the default is required when variable is NOT NULL

>
> > --- /dev/null
> > +++ b/doc/src/sgml/ref/let.sgml
>
> > + <varlistentry>
> > + <term><literal>sql_expression</literal></term>
> > + <listitem>
> > + <para>
> > + An SQL expression. The result is cast into the schema variable's
> type.
> > + </para>
> > + </listitem>
> > + </varlistentry>
>
> Always, even if there is no assignment or implicit cast?
>

It uses implicit cast in COERCION_ASSIGNMENT context. coerce_to_target_type
is used always

This part of doc currently looks

<listitem>
<para>
An SQL expression (can be subquery in parenthesis). The result must
be of castable to the same data type as the session variable (in
implicit or assignment context).
</para>
</listitem>

> I see no such wording fir INSERT or UPDATE, so if only assignment casts
> are used,
> the second sentence should be removed.
>
> > --- a/doc/src/sgml/ref/pg_restore.sgml
> > +++ b/doc/src/sgml/ref/pg_restore.sgml
>
> > + <varlistentry>
> > + <term><option>-A <replaceable
> class="parameter">schema_variable</replaceable></option></term>
> > + <term><option>--variable=<replaceable
> class="parameter">schema_variable</replaceable></option></term>
> > + <listitem>
> > + <para>
> > + Restore a named schema variable only. Multiple schema
> variables may be specified with
> > + multiple <option>-A</option> switches.
> > + </para>
> > + </listitem>
> > + </varlistentry>
>
> Do we need that? We have no such option for functions and other
> non-relations.
>

It is designed to be consistent with others. pg_restore supports functions
-P, triggers -T

>
> And if we really want such an option for "pg_restore", why not for
> "pg_dump"?
>

I have no strong opinion about it, I think so it is consistent with other
non-relations, but it is not important.

I moved this feature to a separate patch. It can be committed optionaly or
later.

pg_restore has options -P, -T, and pg_dump does not have these options.
These options (functionality) can be implemented in pg_dump too, but
unfortunately -T is used for different purposes (exclude table).

Regards

Pavel

>
> Yours,
> Laurenz Albe
>