From 63b0801173f13e098688d685aaec8dfb058363d1 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Wed, 24 Jan 2018 09:03:35 +0100 Subject: [PATCH 2/2] Avoid silently changing volatilty in create function A function cannot be declared both immutable and volatile, the command will error out on conflicting options: create function int42(cstring) returns int42 AS 'int4in' language internal strict immutable volatile; The following statement would however silently create an immutable function due to iscachable in the with() clause being handled last and overwriting the previous volatility setting. create function int42(cstring) returns int42 AS 'int4in' language internal strict volatile with (isstrict, iscachable); With this patch, compute_attributes_with_style() checks if volatility has been defined elsewhere in the statement and errors out in case conflicting options are detected. --- src/backend/commands/functioncmds.c | 27 ++++++++++++++++++++----- src/test/regress/expected/create_function_3.out | 3 +++ src/test/regress/sql/create_function_3.sql | 2 ++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 29d2b3a7ff..b6faa83358 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -651,7 +651,8 @@ compute_attributes_sql_style(ParseState *pstate, ArrayType **proconfig, float4 *procost, float4 *prorows, - char *parallel_p) + char *parallel_p, + bool *has_volatile) { ListCell *option; DefElem *as_item = NULL; @@ -759,7 +760,12 @@ compute_attributes_sql_style(ParseState *pstate, if (windowfunc_item) *windowfunc_p = intVal(windowfunc_item->arg); if (volatility_item) + { + *has_volatile = true; *volatility_p = interpret_func_volatility(volatility_item); + } + else + *has_volatile = false; if (strict_item) *strict_p = intVal(strict_item->arg); if (security_item) @@ -791,7 +797,8 @@ compute_attributes_sql_style(ParseState *pstate, /*------------- * Interpret the parameters *parameters and return their contents via - * *isStrict_p and *volatility_p. + * *isStrict_p and *volatility_p. If has_volatile is true then volatility + * has been defined already in the syntax. * * These parameters supply optional information about a function. * All have defaults if not specified. Parameters: @@ -804,7 +811,7 @@ compute_attributes_sql_style(ParseState *pstate, *------------ */ static void -compute_attributes_with_style(ParseState *pstate, bool is_procedure, List *parameters, bool *isStrict_p, char *volatility_p) +compute_attributes_with_style(ParseState *pstate, bool is_procedure, List *parameters, bool *isStrict_p, char *volatility_p, bool has_volatile) { ListCell *pl; @@ -835,8 +842,15 @@ compute_attributes_with_style(ParseState *pstate, bool is_procedure, List *param (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), errmsg("invalid attribute in procedure definition"), parser_errposition(pstate, param->location))); + if (defGetBoolean(param)) + { + if (has_volatile && *volatility_p != PROVOLATILE_IMMUTABLE) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("conflicting or redundant options"))); *volatility_p = PROVOLATILE_IMMUTABLE; + } } else ereport(WARNING, @@ -944,6 +958,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt) isStrict, security, isLeakProof; + bool hasVolatile; char volatility; ArrayType *proconfig; float4 procost; @@ -981,7 +996,8 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt) &as_clause, &language, &transformDefElem, &isWindowFunc, &volatility, &isStrict, &security, &isLeakProof, - &proconfig, &procost, &prorows, ¶llel); + &proconfig, &procost, &prorows, ¶llel, + &hasVolatile); /* Look up the language and validate permissions */ languageTuple = SearchSysCache1(LANGNAME, PointerGetDatum(language)); @@ -1113,7 +1129,8 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt) trftypes = NULL; } - compute_attributes_with_style(pstate, stmt->is_procedure, stmt->withClause, &isStrict, &volatility); + compute_attributes_with_style(pstate, stmt->is_procedure, stmt->withClause, + &isStrict, &volatility, hasVolatile); interpret_AS_clause(languageOid, language, funcname, as_clause, &prosrc_str, &probin_str); diff --git a/src/test/regress/expected/create_function_3.out b/src/test/regress/expected/create_function_3.out index b5e19485e5..3d522c974c 100644 --- a/src/test/regress/expected/create_function_3.out +++ b/src/test/regress/expected/create_function_3.out @@ -66,6 +66,9 @@ SELECT proname, provolatile FROM pg_proc functest_b_4 | v (4 rows) +CREATE FUNCTION functest_B_5(int) RETURNS bool LANGUAGE 'sql' + VOLATILE AS 'SELECT $1 < 0' WITH (iscachable); +ERROR: conflicting or redundant options -- -- SECURITY DEFINER | INVOKER -- diff --git a/src/test/regress/sql/create_function_3.sql b/src/test/regress/sql/create_function_3.sql index 0a0e407aab..187454b643 100644 --- a/src/test/regress/sql/create_function_3.sql +++ b/src/test/regress/sql/create_function_3.sql @@ -48,6 +48,8 @@ SELECT proname, provolatile FROM pg_proc 'functest_B_2'::regproc, 'functest_B_3'::regproc, 'functest_B_4'::regproc) ORDER BY proname; +CREATE FUNCTION functest_B_5(int) RETURNS bool LANGUAGE 'sql' + VOLATILE AS 'SELECT $1 < 0' WITH (iscachable); -- -- SECURITY DEFINER | INVOKER -- 2.14.1.145.gb3622a4ee