Hi all,

As noticed by Daniel here:
https://www.postgresql.org/message-id/d5f34c9d-3ab7-4419-af2e-12f67581d...@yesql.se

Using a WITH clause takes precendence over what is defined in the main
function definition when using isStrict and isCachable. For example,
when using VOLATILE and IMMUTABLE, an error is raised:
=# create function int42(cstring) returns int42 AS 'int4in'
   language internal strict immutable volatile;
ERROR:  42601: conflicting or redundant options
LINE 2:     language internal strict immutable volatile;

However when using for example STABLE/VOLATILE in combination with a
WITH clause, then things get prioritized, and in this case the WITH
clause values are taken into account:
=# create function int42(cstring) returns int42 AS 'int4in'
   language internal strict volatile with (isstrict, iscachable);
CREATE FUNCTION
=# select provolatile from pg_proc where proname = 'int42';
 provolatile
-------------
 i
(1 row)

This clause is marked as deprecated since 7.3, so perhaps it would be
time to remove completely its support? It seems to me that this leads to
more confusion than being helpful. And I have not found a trace of code
using those flags on github or such.

Thanks,
--
Michael
From 99a1881bfe605dcb57daab2cb38ef1c7d2805f01 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 15 Jan 2018 11:20:03 +0900
Subject: [PATCH] Remove support for WITH clause in CREATE FUNCTION

The WITH clause in CREATE FUNCTION has been able to use two parameters,
which have been marked as deprecated in 7.3:
- isStrict, which is equivalent to STRICT
- isCachable, which is equivalent to IMMUTABLE

Specifying in parallel multiple values of IMMUTABLE/STABLE/VOLATILE
results in an error, but using a WITH clause with other keywords gives
priority to the values in the WITH clause, which can be confusing for
users.
---
 doc/src/sgml/ref/create_function.sgml | 36 -----------------------
 src/backend/commands/functioncmds.c   | 55 -----------------------------------
 src/backend/nodes/copyfuncs.c         |  1 -
 src/backend/nodes/equalfuncs.c        |  1 -
 src/backend/parser/gram.y             |  9 ++----
 src/include/nodes/parsenodes.h        |  1 -
 6 files changed, 3 insertions(+), 100 deletions(-)

diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml
index fd229d1193..c0adb8cf1e 100644
--- a/doc/src/sgml/ref/create_function.sgml
+++ b/doc/src/sgml/ref/create_function.sgml
@@ -37,7 +37,6 @@ CREATE [ OR REPLACE ] FUNCTION
     | AS '<replaceable class="parameter">definition</replaceable>'
     | AS '<replaceable class="parameter">obj_file</replaceable>', '<replaceable class="parameter">link_symbol</replaceable>'
   } ...
-    [ WITH ( <replaceable class="parameter">attribute</replaceable> [, ...] ) ]
 </synopsis>
  </refsynopsisdiv>
 
@@ -560,41 +559,6 @@ CREATE [ OR REPLACE ] FUNCTION
      </listitem>
     </varlistentry>
 
-    <varlistentry>
-     <term><replaceable class="parameter">attribute</replaceable></term>
-
-     <listitem>
-      <para>
-       The historical way to specify optional pieces of information
-       about the function.  The following attributes can appear here:
-
-      <variablelist>
-       <varlistentry>
-        <term><literal>isStrict</literal></term>
-        <listitem>
-         <para>
-          Equivalent to <literal>STRICT</literal> or <literal>RETURNS NULL ON NULL INPUT</literal>.
-         </para>
-        </listitem>
-       </varlistentry>
-
-       <varlistentry>
-        <term><literal>isCachable</literal></term>
-        <listitem>
-         <para><literal>isCachable</literal> is an obsolete equivalent of
-          <literal>IMMUTABLE</literal>; it's still accepted for
-          backwards-compatibility reasons.
-         </para>
-        </listitem>
-       </varlistentry>
-
-      </variablelist>
-
-      Attribute names are not case-sensitive.
-     </para>
-    </listitem>
-   </varlistentry>
-
    </variablelist>
 
    <para>
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 12ab33f418..347e25db10 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -788,59 +788,6 @@ compute_attributes_sql_style(ParseState *pstate,
 }
 
 
-/*-------------
- *	 Interpret the parameters *parameters and return their contents via
- *	 *isStrict_p and *volatility_p.
- *
- *	These parameters supply optional information about a function.
- *	All have defaults if not specified. Parameters:
- *
- *	 * isStrict means the function should not be called when any NULL
- *	   inputs are present; instead a NULL result value should be assumed.
- *
- *	 * volatility tells the optimizer whether the function's result can
- *	   be assumed to be repeatable over multiple evaluations.
- *------------
- */
-static void
-compute_attributes_with_style(ParseState *pstate, bool is_procedure, List *parameters, bool *isStrict_p, char *volatility_p)
-{
-	ListCell   *pl;
-
-	foreach(pl, parameters)
-	{
-		DefElem    *param = (DefElem *) lfirst(pl);
-
-		if (pg_strcasecmp(param->defname, "isstrict") == 0)
-		{
-			if (is_procedure)
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
-						 errmsg("invalid attribute in procedure definition"),
-						 parser_errposition(pstate, param->location)));
-			*isStrict_p = defGetBoolean(param);
-		}
-		else if (pg_strcasecmp(param->defname, "iscachable") == 0)
-		{
-			/* obsolete spelling of isImmutable */
-			if (is_procedure)
-				ereport(ERROR,
-						(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
-						 errmsg("invalid attribute in procedure definition"),
-						 parser_errposition(pstate, param->location)));
-			if (defGetBoolean(param))
-				*volatility_p = PROVOLATILE_IMMUTABLE;
-		}
-		else
-			ereport(WARNING,
-					(errcode(ERRCODE_SYNTAX_ERROR),
-					 errmsg("unrecognized function attribute \"%s\" ignored",
-							param->defname),
-					 parser_errposition(pstate, param->location)));
-	}
-}
-
-
 /*
  * For a dynamically linked C language object, the form of the clause is
  *
@@ -1106,8 +1053,6 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
 		trftypes = NULL;
 	}
 
-	compute_attributes_with_style(pstate, stmt->is_procedure, stmt->withClause, &isStrict, &volatility);
-
 	interpret_AS_clause(languageOid, language, funcname, as_clause,
 						&prosrc_str, &probin_str);
 
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index ddbbc79823..307170c2f6 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3425,7 +3425,6 @@ _copyCreateFunctionStmt(const CreateFunctionStmt *from)
 	COPY_NODE_FIELD(returnType);
 	COPY_SCALAR_FIELD(is_procedure);
 	COPY_NODE_FIELD(options);
-	COPY_NODE_FIELD(withClause);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 30ccc9c5ae..d2092b77b0 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1374,7 +1374,6 @@ _equalCreateFunctionStmt(const CreateFunctionStmt *a, const CreateFunctionStmt *
 	COMPARE_NODE_FIELD(returnType);
 	COMPARE_SCALAR_FIELD(is_procedure);
 	COMPARE_NODE_FIELD(options);
-	COMPARE_NODE_FIELD(withClause);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e42b7caff6..c099b4d6bb 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -7479,7 +7479,7 @@ opt_nulls_order: NULLS_LA FIRST_P			{ $$ = SORTBY_NULLS_FIRST; }
 
 CreateFunctionStmt:
 			CREATE opt_or_replace FUNCTION func_name func_args_with_defaults
-			RETURNS func_return createfunc_opt_list opt_definition
+			RETURNS func_return createfunc_opt_list
 				{
 					CreateFunctionStmt *n = makeNode(CreateFunctionStmt);
 					n->replace = $2;
@@ -7487,11 +7487,10 @@ CreateFunctionStmt:
 					n->parameters = $5;
 					n->returnType = $7;
 					n->options = $8;
-					n->withClause = $9;
 					$$ = (Node *)n;
 				}
 			| CREATE opt_or_replace FUNCTION func_name func_args_with_defaults
-			  RETURNS TABLE '(' table_func_column_list ')' createfunc_opt_list opt_definition
+			  RETURNS TABLE '(' table_func_column_list ')' createfunc_opt_list
 				{
 					CreateFunctionStmt *n = makeNode(CreateFunctionStmt);
 					n->replace = $2;
@@ -7500,11 +7499,10 @@ CreateFunctionStmt:
 					n->returnType = TableFuncTypeName($9);
 					n->returnType->location = @7;
 					n->options = $11;
-					n->withClause = $12;
 					$$ = (Node *)n;
 				}
 			| CREATE opt_or_replace FUNCTION func_name func_args_with_defaults
-			  createfunc_opt_list opt_definition
+			  createfunc_opt_list
 				{
 					CreateFunctionStmt *n = makeNode(CreateFunctionStmt);
 					n->replace = $2;
@@ -7512,7 +7510,6 @@ CreateFunctionStmt:
 					n->parameters = $5;
 					n->returnType = NULL;
 					n->options = $6;
-					n->withClause = $7;
 					$$ = (Node *)n;
 				}
 			| CREATE opt_or_replace PROCEDURE func_name func_args_with_defaults
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index b72178efd1..7ee4002667 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2755,7 +2755,6 @@ typedef struct CreateFunctionStmt
 	TypeName   *returnType;		/* the return type */
 	bool		is_procedure;
 	List	   *options;		/* a list of DefElem */
-	List	   *withClause;		/* a list of DefElem */
 } CreateFunctionStmt;
 
 typedef enum FunctionParameterMode
-- 
2.11.0

Attachment: signature.asc
Description: PGP signature

Reply via email to