public inbox for [email protected]
help / color / mirror / Atom feedFrom: Yura Sokolov <[email protected]>
To: [email protected]
Subject: When IMMUTABLE is not.
Date: Thu, 15 Jun 2023 13:22:28 +0300
Message-ID: <[email protected]> (raw)
Good day, hackers.
I found, than declaration of function as IMMUTABLE/STABLE is not enough to be sure
function doesn't manipulate data.
In fact, SPI checks only direct function kind, but fails to check indirect call.
Attached immutable_not.sql creates 3 functions:
- `immutable_direct` is IMMUTABLE and tries to insert into table directly.
PostgreSQL correctly detects and forbids this action.
- `volatile_direct` is VOLATILE and inserts into table directly.
It is allowed and executed well.
- `immutable_indirect` is IMMUTABLE and calls `volatile_direct`.
PostgreSQL failed to detect and prevent this DML manipulation.
Output:
select immutable_direct('immutable_direct'); psql:immutable_not.sql:28:
ERROR: INSERT is not allowed in a non-volatile function CONTEXT: SQL
statement "insert into xxx values(j)" PL/pgSQL function
immutable_direct(character varying) line 3 at SQL statement select
volatile_direct('volatile_direct'); volatile_direct -----------------
volatile_direct (1 row) select immutable_indirect('immutable_indirect');
immutable_indirect -------------------- immutable_indirect (1 row)
select * from xxx; i -------------------- volatile_direct
immutable_indirect (2 rows) Attached forbid-non-volatile-mutations.diff
add checks readonly function didn't made data manipulations. Output for
patched version: select immutable_indirect('immutable_indirect');
psql:immutable_not.sql:32: ERROR: Damn2! Update were done in a
non-volatile function CONTEXT: SQL statement "SELECT
volatile_direct(j)" PL/pgSQL function immutable_indirect(character
varying) line 3 at PERFORM I doubt check should be done this way. This
check is necessary, but it should be FATAL instead of ERROR. And ERROR
should be generated at same place, when it is generated for
`immutable_direct`, but with check of "read_only" status through whole
call stack instead of just direct function kind. ----- regards, Yura
Sokolov Postgres Professional
Attachments:
[application/sql] immutable_not.sql (691B, ../[email protected]/2-immutable_not.sql)
download
[text/x-patch] forbid-non-volatile-mutations.diff (2.2K, ../[email protected]/3-forbid-non-volatile-mutations.diff)
download | inline diff:
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 33975687b38..82b6127d650 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1584,6 +1584,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
Portal portal;
SPICallbackArg spicallbackarg;
ErrorContextCallback spierrcontext;
+ CommandId this_command = InvalidCommandId;
/*
* Check that the plan is something the Portal code will special-case as
@@ -1730,6 +1731,7 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
if (read_only)
{
ListCell *lc;
+ this_command = GetCurrentCommandId(false);
foreach(lc, stmt_list)
{
@@ -1778,6 +1780,14 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
/* Pop the SPI stack */
_SPI_end_call(true);
+ if (read_only && this_command != GetCurrentCommandId(false))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ /* translator: %s is a SQL statement name */
+ errmsg("Damn1! Update were done in a non-volatile function")));
+ }
+
/* Return the created portal */
return portal;
}
@@ -2404,6 +2414,7 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
ErrorContextCallback spierrcontext;
CachedPlan *cplan = NULL;
ListCell *lc1;
+ CommandId this_command = InvalidCommandId;
/*
* Setup error traceback support for ereport()
@@ -2473,6 +2484,11 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("empty query does not return tuples")));
+ if (options->read_only)
+ {
+ this_command = GetCurrentCommandId(false);
+ }
+
foreach(lc1, plan->plancache_list)
{
CachedPlanSource *plansource = (CachedPlanSource *) lfirst(lc1);
@@ -2788,6 +2804,14 @@ _SPI_execute_plan(SPIPlanPtr plan, const SPIExecuteOptions *options,
CommandCounterIncrement();
}
+ if (options->read_only && this_command != GetCurrentCommandId(false))
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ /* translator: %s is a SQL statement name */
+ errmsg("Damn2! Update were done in a non-volatile function")));
+ }
+
fail:
/* Pop the snapshot off the stack if we pushed one */
view thread (3+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected]
Subject: Re: When IMMUTABLE is not.
In-Reply-To: <[email protected]>
* 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