Re: proposal: schema variables - Mailing list pgsql-hackers
From | Laurenz Albe |
---|---|
Subject | Re: proposal: schema variables |
Date | |
Msg-id | 9e67d49deb18270eddb95e602c83f02b98459843.camel@cybertec.at Whole thread Raw |
In response to | Re: proposal: schema variables (Pavel Stehule <pavel.stehule@gmail.com>) |
Responses |
Re: proposal: schema variables
Re: proposal: schema variables |
List | pgsql-hackers |
Thanks for the fixes and the new patch set! I think that this would be a very valuable feature! This is a very incomplete review after playing with the patch set for a while. Some bugs and oddities I have found: "psql" support: \? gives \dV [PATTERN] list variables I think that should say "schema variables" to distinguish them from psql variables. Running \dV when connected to an older server gives ERROR: relation "pg_catalog.pg_variable" does not exist LINE 16: FROM pg_catalog.pg_variable v ^ I think it would be better not to run the query and show a response like session variables don't exist in server version 16 The LET statement: CREATE VARIABLE testvar AS int4multirange[]; LET testvar = '{\{[2\,7]\,[11\,13]\}}'; ERROR: variable "laurenz.testvar" is of type int4multirange[], but expression is of type text LINE 1: LET testvar = '{\{[2\,7]\,[11\,13]\}}'; ^ HINT: You will need to rewrite or cast the expression. Sure, I can add an explicit type cast, but I think that the type should be determined by the type of the variable. The right-hand side should be treated as "unknown", and the type input function should be used. Parameter session_variables_ambiguity_warning: --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -1544,6 +1544,16 @@ struct config_bool ConfigureNamesBool[] = false, NULL, NULL, NULL }, + { + {"session_variables_ambiguity_warning", PGC_USERSET, DEVELOPER_OPTIONS, + gettext_noop("Raise warning when reference to a session variable is ambiguous."), + NULL, + GUC_NOT_IN_SAMPLE + }, + &session_variables_ambiguity_warning, + false, + NULL, NULL, NULL + }, I think the short_desc should be "Raise a warning" (with the indefinite article). DEVELOPER_OPTIONS is the wrong category. We normally use that for parameters that are only relevant for PostgreSQL hackers. I think it should be CLIENT_CONN_OTHER. CREATE VARIABLE command: CREATE VARIABLE str AS text NOT NULL; ERROR: session variable must have a default value, since it's declared NOT NULL Perhaps this error message would be better: session variables declared as NOT NULL must have a default value This is buggy: CREATE VARIABLE str AS text NOT NULL DEFAULT NULL; Ugh. SELECT str; ERROR: null value is not allowed for NOT NULL session variable "laurenz.str" DETAIL: The result of DEFAULT expression is NULL. Perhaps that is a leftover from the previous coding, but I think there need be no check upon SELECT. It should be enough to check during CREATE VARIABLE and LET. pg_dump support: The attempt to dump a database with an older version leads to pg_dump: error: query failed: ERROR: relation "pg_catalog.pg_variable" does not exist LINE 14: FROM pg_catalog.pg_variable v ^ Dumping variables must be conditional on the server version. IMMUTABLE variables: + <varlistentry id="sql-createvariable-immutable"> + <term><literal>IMMUTABLE</literal></term> + <listitem> + <para> + The assigned value of the session variable can not be changed. + Only if the session variable doesn't have a default value, a single + initialization is allowed using the <command>LET</command> command. Once + done, no further change is allowed until end of transaction + if the session variable was created with clause <literal>ON TRANSACTION + END RESET</literal>, or until reset of all session variables by + <command>DISCARD VARIABLES</command>, or until reset of all session + objects by command <command>DISCARD ALL</command>. + </para> + </listitem> + </varlistentry> I can see the usefulness of IMMUTABLE variables, but I am surprised that they are reset by DISCARD. What is the use case you have in mind? The use case I can envision is an application that sets a value right after authentication, for use with row-level security. But then it would be harmful if the user could reset the variable with DISCARD. Documentation: --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml + <para> + The session variables can be shadowed by column references in a query. When + a query contains identifiers or qualified identifiers that could be used as + both a session variable identifiers and as column identifier, then the + column identifier is preferred every time. Warnings can be emitted when + this situation happens by enabling configuration parameter <xref + linkend="guc-session-variables-ambiguity-warning"/>. User can explicitly + qualify the source object by syntax <literal>table.column</literal> or + <literal>variable.column</literal>. + </para> I think you mean <literal>schema.variable</literal>, not <literal>variable.column</literal>. Yours, Laurenz Albe
pgsql-hackers by date: