public inbox for pgsql-performance@postgresql.org  
help / color / mirror / Atom feed
From: Pavel Stehule <pavel.stehule@gmail.com>
To: DUVAL REMI <REMI.DUVAL@cheops.fr>
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: phb07@apra.asso.fr <phb07@apra.asso.fr>
Subject: Re: proposal: schema variables
Date: Fri, 20 Mar 2020 08:18:45 +0100
Message-ID: <CAFj8pRC_sMuOjUn=ikNavENVRqFZahBrAiAqiMfjMf1kUj7+aw@mail.gmail.com> (raw)
In-Reply-To: <50205b58f0644cff9f5fe56ea5e973a2@CHG-E2K13-DC2.INTRANET.CHEOPS.FR>
References: <CAFj8pRDY+m9OOxfO10R7J0PAkCCauM-TweaTrdsrsLGMb1VbEQ@mail.gmail.com>
	<CAFj8pRBQSbOvK94QJCZGY7xqiuT7R7tuGKgci+CXGYONs5PGOw@mail.gmail.com>
	<158272883575.1651.2316685587547965508.pgcf@coridan.postgresql.org>
	<CAFj8pRDj7ss7YNRdF1nD6=DaeAk7YJ=Nz6449-g0DrPEZmxEug@mail.gmail.com>
	<CAFj8pRC9H8s3=Twpk4e+As6Sz7NrceynEKsupg5JLNpVBe4sqQ@mail.gmail.com>
	<CAFj8pRDZ-SxOwfupyD=YhGAMKjNetWhGVkdq9RUs-44wjpmxPQ@mail.gmail.com>
	<CAFj8pRALAZOu6bXTaz6CHox9S=OqHT-suX+LRgA4z0nzhXv4Vw@mail.gmail.com>
	<CADM=Jej3onf9VK_3BfsuCpRLnXrYKp+cCY2PtahpCXRY4jG1iw@mail.gmail.com>
	<CAFj8pRCzjcYXTOgOoM41RwVr38mPf172_A184pPLeybA_QvqpQ@mail.gmail.com>
	<158cad51e4004c70ab621af77edae2ee@CHG-E2K13-DC2.INTRANET.CHEOPS.FR>
	<CAFj8pRDaGtWaTY5PD4Hp+yuzxhuseqq=N3HMVv0hAza2srKEvw@mail.gmail.com>
	<CAFj8pRD8T5os8Yym+mKjHKQzntS8fQkS+GoK8sKQmvHgF_z=hg@mail.gmail.com>
	<CAFj8pRDepLHgceFPqNYTtkx0DgbdxsRP=G3+yL8mCW0K8=81Tg@mail.gmail.com>
	<CAFj8pRDnxjje-FAc9bRFtt3Di=ZhGi6DOVAEZnHDesfR8zmN-Q@mail.gmail.com>
	<CAFj8pRBC18eAfiW0wM=RYr0nW+jr-Z3M=stPWaX4W7SYasT0NQ@mail.gmail.com>
	<3914c1f26c744da0afa8499a2020cc27@CHG-E2K13-DC2.INTRANET.CHEOPS.FR>
	<CAFj8pRDcOAyYSChONX5T-Ow35iEw5aEXOAX8nqKPNJ9mUbmDYA@mail.gmail.com>
	<f6d7b06e59ac44c29ca2f93e403b5ddd@CHG-E2K13-DC2.INTRANET.CHEOPS.FR>
	<CAFj8pRARhPc_wqQ2EP-BNOnMFia6Yt=X+YhYJRs2dG+ebmUq+Q@mail.gmail.com>
	<7e2099c6bf794408b5f49fcdc2a8136d@CHG-E2K13-DC2.INTRANET.CHEOPS.FR>
	<CAFj8pRBWg0xqNqnpRz7hSLNcB0wxrfZ2n-7k-nfX=o=mJ24O-A@mail.gmail.com>
	<50205b58f0644cff9f5fe56ea5e973a2@CHG-E2K13-DC2.INTRANET.CHEOPS.FR>

čt 19. 3. 2020 v 10:43 odesílatel DUVAL REMI <REMI.DUVAL@cheops.fr> napsal:

> Hello
>
>
>
> I played around with the ALTER VARIABLE statement to make sure it’s OK and
> it seems fine to me.
>
>
>
> Another last thing before commiting.
>
>
>
> I agree with Thomas Vondra, that this part might/should be simplified :
>
>
>
> [ { ON COMMIT DROP | ON { TRANSACTIONAL | TRANSACTION } END RESET } ]
>
>
>
> I would only allow “ON TRANSACTION END RESET”.
>
> I think we don’t need both here.
>
> Philippe Beaudoin was indeed talking about the TRANSACTIONAL keyword, but
> that would have make sense (and I think that’s what he meant) , if you
> could do something like “CREATE [NON] TRANSACTIONAL VARIABLE …”.
>
> But here I don’t think that the ON TRANSACTIONAL END RESET has any sense
> in English, and it only complicates the syntax.
>
>
>
> Maybe Thomas Vondra (if it’s him) would be more inclined to commit the
> patch if it has this more simple syntax has he requested.
>
>
>
> What do you think ?
>

I removed TRANSACTIONAL from this clause, see attachement change.diff

Updated patch attached.

I hope it would be the last touch, making it fully ready for a commiter.
>

Thank you very much for review and testing

Pavel

>


Attachments:

  [text/x-patch] change.diff (2.0K, 3-change.diff)
  download | inline diff:
diff --git a/doc/src/sgml/ref/create_variable.sgml b/doc/src/sgml/ref/create_variable.sgml
index 1b1883a11a..cf175e05c6 100644
--- a/doc/src/sgml/ref/create_variable.sgml
+++ b/doc/src/sgml/ref/create_variable.sgml
@@ -22,7 +22,7 @@ PostgreSQL documentation
  <refsynopsisdiv>
 <synopsis>
 CREATE [ { TEMPORARY | TEMP } ] [ IMMUTABLE ] VARIABLE [ IF NOT EXISTS ] <replaceable class="parameter">name</replaceable> [ AS ] <replaceable class="parameter">data_type</replaceable> ] [ COLLATE <replaceable class="parameter">collation</replaceable> ]
-    [ NOT NULL ] [ DEFAULT <replaceable class="parameter">default_expr</replaceable> ] [ { ON COMMIT DROP | ON { TRANSACTIONAL | TRANSACTION } END RESET } ]
+    [ NOT NULL ] [ DEFAULT <replaceable class="parameter">default_expr</replaceable> ] [ { ON COMMIT DROP | ON { TRANSACTION } END RESET } ]
 </synopsis>
  </refsynopsisdiv>
  <refsect1>
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 744342733d..c135a35d07 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4567,7 +4567,6 @@ OptSchemaVarDefExpr: DEFAULT b_expr					{ $$ = $2; }
 
 OnEOXActionOption:  ON COMMIT DROP					{ $$ = VARIABLE_EOX_DROP; }
 			| ON TRANSACTION END_P RESET			{ $$ = VARIABLE_EOX_RESET; }
-			| ON TRANSACTIONAL END_P RESET			{ $$ = VARIABLE_EOX_RESET; }
 			| /*EMPTY*/								{ $$ = VARIABLE_EOX_NOOP; }
 		;
 
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index d39bcfe9cf..d26b0efcd9 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5978,7 +5978,7 @@
   proname => 'pg_collation_is_visible', procost => '10', provolatile => 's',
   prorettype => 'bool', proargtypes => 'oid',
   prosrc => 'pg_collation_is_visible' },
-{ oid => '4191', descr => 'is schema variable visible in search path?',
+{ oid => '4142', descr => 'is schema variable visible in search path?',
   proname => 'pg_variable_is_visible', procost => '10', provolatile => 's',
   prorettype => 'bool', proargtypes => 'oid', prosrc => 'pg_variable_is_visible' },
 


  [application/gzip] schema-variables-20200320.patch.gz (66.2K, 4-schema-variables-20200320.patch.gz)
  download

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-performance@postgresql.org
  Cc: pavel.stehule@gmail.com, REMI.DUVAL@cheops.fr, pgsql-hackers@lists.postgresql.org, phb07@apra.asso.fr
  Subject: Re: proposal: schema variables
  In-Reply-To: <CAFj8pRC_sMuOjUn=ikNavENVRqFZahBrAiAqiMfjMf1kUj7+aw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox