From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> |
Cc: | Erik Rijkers <er(at)xs4all(dot)nl>, Michael Paquier <michael(at)paquier(dot)xyz>, Zhihong Yu <zyu(at)yugabyte(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, DUVAL REMI <REMI(dot)DUVAL(at)cheops(dot)fr>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: proposal: schema variables |
Date: | 2024-07-22 06:37:22 |
Message-ID: | CAFj8pRBaD0_bMrCREWnVLfcTMdc0v7ns7Rt=sEvd1EoFmLfarQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-performance |
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
>
From | Date | Subject | |
---|---|---|---|
Next Message | Will Mortensen | 2024-07-22 06:46:51 | Re: Exposing the lock manager's WaitForLockers() to SQL |
Previous Message | kuroda.keisuke | 2024-07-22 06:23:41 | Re: Add privileges test for pg_stat_statements to improve coverage |
From | Date | Subject | |
---|---|---|---|
Next Message | Laurenz Albe | 2024-07-22 08:23:39 | Re: proposal: schema variables |
Previous Message | Pavel Stehule | 2024-07-20 19:48:43 | Re: proposal: schema variables |