Обсуждение: Add notification on BEGIN ATOMIC SQL functions using temp relations

Hi,

While reviewing a patch I noticed that SQL functions defined with BEGIN
ATOMIC can reference temporary relations, and such functions are
(rightfully) dropped at session end --- but without any notification to
the user:

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

postgres=# CREATE TEMPORARY TABLE tmp AS SELECT 42 AS val;
SELECT 1

postgres=# CREATE FUNCTION tmpval_atomic()
RETURNS int LANGUAGE sql
BEGIN ATOMIC;
SELECT val FROM tmp;
END;
CREATE FUNCTION

postgres=# \df
                           List of functions
 Schema |     Name      | Result data type | Argument data types | Type
--------+---------------+------------------+---------------------+------
 public | tmpval_atomic | integer          |                     | func
(1 row)

postgres=# \q

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

postgres=# \df
                       List of functions
 Schema | Name | Result data type | Argument data types | Type
--------+------+------------------+---------------------+------
(0 rows)


Although this behaviour is expected, it can be surprising. A NOTICE or
WARNING at CREATE FUNCTION time could save some head-scratching later.
We already have a precedent. When creating a view that depends on a
temporary relation, postgres automatically makes it a temporary view and
emits a NOTICE:

postgres=# CREATE TEMPORARY TABLE tmp AS SELECT 42 AS val;
SELECT 1

postgres=# CREATE VIEW v AS SELECT * FROM tmp;
NOTICE:  view "v" will be a temporary view
CREATE VIEW

postgres=# \d
         List of relations
   Schema   | Name | Type  | Owner
------------+------+-------+-------
 pg_temp_74 | tmp  | table | jim
 pg_temp_74 | v    | view  | jim
(2 rows)

postgres=# \q

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

postgres=# \d
Did not find any relations.


Attached a PoC that issues a WARNING if a BEGIN ATOMIC function is
created using temporary objects:

postgres=# CREATE TEMPORARY TABLE tmp AS SELECT 42 AS val;
SELECT 1

postgres=# CREATE FUNCTION tmpval_atomic()
RETURNS int LANGUAGE sql
BEGIN ATOMIC;
SELECT val FROM tmp;
END;
WARNING:  function defined with BEGIN ATOMIC depends on temporary
relation "tmp"
DETAIL:  the function will be dropped automatically at session end.
CREATE FUNCTION

This PoC adds a parameter to check_sql_fn_statements() and
check_sql_fn_statement(), so I’m not entirely sure if that’s the best
approach. I’m also not sure whether a NOTICE would be a better fit than
a WARNING here. Feedback is welcome.

Any thoughts?

Best regards, Jim

Hi Pavel

On 9/21/25 14:33, Pavel Stehule wrote:
> i understand your motivation, but with this warning temp tables cannot
> be used in SQL function due log overhead.

My intention was not to warn on every function call. The WARNING is only
emitted once at CREATE FUNCTION time, similar to how CREATE VIEW warns
if a view depends on a temporary relation. After creation, the function
can still be used normally without additional log overhead. Is there a
side effect I might be overlooking here?

postgres=# CREATE TEMPORARY TABLE tmp AS SELECT 42 AS val;
SELECT 1

postgres=# CREATE FUNCTION tmpval_atomic()
RETURNS int LANGUAGE sql
BEGIN ATOMIC;
SELECT val FROM tmp;
END;
WARNING:  function defined with BEGIN ATOMIC depends on temporary
relation "tmp"
DETAIL:  the function will be dropped automatically at session end.
CREATE FUNCTION

postgres=# SELECT tmpval_atomic();
 tmpval_atomic
---------------
            42
(1 row)

Best regards, Jim

Hi Pavel

On 9/21/25 14:33, Pavel Stehule wrote:
> i understand your motivation, but with this warning temp tables cannot
> be used in SQL function due log overhead.

My intention was not to warn on every function call. The WARNING is only
emitted once at CREATE FUNCTION time, similar to how CREATE VIEW warns
if a view depends on a temporary relation. After creation, the function
can still be used normally without additional log overhead. Is there a
side effect I might be overlooking here?

On 21/09/2025 13:49, Jim Jones wrote:
> WARNING:  function defined with BEGIN ATOMIC depends on temporary
> relation "tmp"
> DETAIL:  the function will be dropped automatically at session end.
> CREATE FUNCTION


In addition to what others have said, this DETAIL line needs to be 
contextual.  The temporary table could have been declared as ON COMMIT 
DROP in which case the function will only last until transaction end.

-- 

Vik Fearing

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> I’m surprised that this is how the system works and I agree that either we
> should add this notice or remove the one for create view.  Even more
> because there is no syntax for directly creating a temporary function -

It is possible to do

CREATE FUNCTION pg_temp.foo() ...

However, then it's not in your search path and you have to write
"pg_temp.foo" to call it, so this is far from transparent.

The fact that you can't call a temporary function without explicit
schema qualification is a security decision that is very unlikely
to get relaxed.  But because of that, temp functions aren't really
first-class objects, and so I wouldn't be in favor of inventing
CREATE TEMP FUNCTION.

There's a larger issue here though: a function such as Jim shows
is a normal function, probably stored in the public schema, and
by default other sessions will be able to call it.  But it will
certainly not work as desired for them, since they can't access
the creating session's temp tables.  It would likely bollix
a concurrent pg_dump too.  I wonder if we'd be better off to
forbid creation of such a function altogether.

            regards, tom lane

On 9/21/25 16:59, Tom Lane wrote:
> There's a larger issue here though: a function such as Jim shows
> is a normal function, probably stored in the public schema, and
> by default other sessions will be able to call it.  But it will
> certainly not work as desired for them, since they can't access
> the creating session's temp tables.  It would likely bollix
> a concurrent pg_dump too.  I wonder if we'd be better off to
> forbid creation of such a function altogether.

That's indeed a much larger problem. Calling it from a session silently
delivers a "wrong" result --- I was expecting an error.

== Session 1 ==

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

postgres=#
postgres=# CREATE TEMPORARY TABLE tmp AS SELECT 42 AS val;
SELECT 1
postgres=# CREATE FUNCTION f()
RETURNS int LANGUAGE sql
BEGIN ATOMIC;
SELECT val FROM tmp;
END;
CREATE FUNCTION
postgres=# SELECT f();
 f
----
 42
(1 row)

== Session 2 (concurrent) ==

$ /usr/local/postgres-dev/bin/psql postgres
psql (19devel)
Type "help" for help.

postgres=# SELECT f();
 f
---

(1 row)


In that light, forbidding creation of functions that depend on temporary
objects might be the safer and more consistent approach.

Best regards, Jim

On 9/21/25 17:37, Jim Jones wrote:
> 
> 
> On 9/21/25 16:59, Tom Lane wrote:
>> There's a larger issue here though: a function such as Jim shows
>> is a normal function, probably stored in the public schema, and
>> by default other sessions will be able to call it.  But it will
>> certainly not work as desired for them, since they can't access
>> the creating session's temp tables.  It would likely bollix
>> a concurrent pg_dump too.  I wonder if we'd be better off to
>> forbid creation of such a function altogether.
> 
> That's indeed a much larger problem. Calling it from a session silently
> delivers a "wrong" result --- I was expecting an error.
> 
> == Session 1 ==
> 
> $ /usr/local/postgres-dev/bin/psql postgres
> psql (19devel)
> Type "help" for help.
> 
> postgres=#
> postgres=# CREATE TEMPORARY TABLE tmp AS SELECT 42 AS val;
> SELECT 1
> postgres=# CREATE FUNCTION f()
> RETURNS int LANGUAGE sql
> BEGIN ATOMIC;
> SELECT val FROM tmp;
> END;
> CREATE FUNCTION
> postgres=# SELECT f();
>  f
> ----
>  42
> (1 row)
> 
> == Session 2 (concurrent) ==
> 
> $ /usr/local/postgres-dev/bin/psql postgres
> psql (19devel)
> Type "help" for help.
> 
> postgres=# SELECT f();
>  f
> ---
> 
> (1 row)
> 
> 
> In that light, forbidding creation of functions that depend on temporary
> objects might be the safer and more consistent approach.
> 
As Tom pointed out, pg_dump produces strange output in this case: it
shows a reference to a temporary table that shouldn’t even be visible:

...

--
-- Name: f(); Type: FUNCTION; Schema: public; Owner: jim
--

CREATE FUNCTION public.f() RETURNS integer
    LANGUAGE sql
    BEGIN ATOMIC
 SELECT tmp.val
    FROM pg_temp_3.tmp;
END;

...

This seems to confirm that allowing such functions leads to more than
just user confusion --- it creates broken dump/restore behaviour.

Given that, I agree forbidding functions from referencing temporary
relations is probably the right fix. If there's consensus, I can rework
my PoC in that direction.


Best regards, Jim

Jim Jones <jim.jones@uni-muenster.de> writes:
> That's indeed a much larger problem. Calling it from a session silently
> delivers a "wrong" result --- I was expecting an error.

Yeah, me too.  See

https://www.postgresql.org/message-id/2736425.1758475979%40sss.pgh.pa.us

            regards, tom lane

On 9/21/25 19:34, Tom Lane wrote:
> Jim Jones <jim.jones@uni-muenster.de> writes:
>> That's indeed a much larger problem. Calling it from a session silently
>> delivers a "wrong" result --- I was expecting an error.
> 
> Yeah, me too.  See
> 
> https://www.postgresql.org/message-id/2736425.1758475979%40sss.pgh.pa.us
> 

The attached PoC now raises an ERROR instead of a WARNING.

A boolean is now computed in fmgr_sql_validator(), set to true if the
function has a prosqlbody (BEGIN ATOMIC) and is defined in a
non-temporary schema. This flag is then used to call
check_sql_fn_statements().

In check_sql_fn_statements(): if the new flag is true, it scans the
function body and raises an error if any temporary relations are found;
if it's false, it skips that check.

In returning.sql there was a query that creates a BEGIN ATOMIC function
using on a temporary table. I changed the table to permanent.

Best regards, Jim