Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit906b682

Browse files
committed
Fix re-execution of a failed SQLFunctionCache entry.
If we error out during execution of a SQL-language function, we willoften leave behind non-null pointers in its SQLFunctionCache's cplanand eslist fields. This is problematic if the SQLFunctionCache isre-used, because those pointers will point at resources that werereleased during error cleanup. This problem escaped detection so farbecause ordinarily we won't re-use an FmgrInfo+SQLFunctionCache structafter a query error. However, in the rather improbable case thatsomeone implements an opclass support function in SQL language, therewill be long-lived FmgrInfos for it in the relcache, and then theproblem is reachable after the function throws an error.To fix, add a flag to SQLFunctionCache that tracks whether executionescapes out of fmgr_sql, and clear out the relevant fields duringinit_sql_fcache if so. (This is going to need more thought if we evertry to share FMgrInfos across threads; but it's very far from beingthe only problem such a project will encounter, since many functionsregard fn_extra as being query-local state.)This broke at commit0313c5d; before that we did not try to re-useSQLFunctionCache state across calls. Hence, back-patch to v18.Bug: #19026Reported-by: Alexander Lakhin <exclusion@gmail.com>Author: Tom Lane <tgl@sss.pgh.pa.us>Discussion:https://postgr.es/m/19026-90aed5e71d0c8af3@postgresql.orgBackpatch-through: 18
1 parentea1c6b0 commit906b682

File tree

3 files changed

+66
-1
lines changed

3 files changed

+66
-1
lines changed

‎src/backend/executor/functions.c‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ typedef struct SQLFunctionCache
143143
{
144144
SQLFunctionHashEntry*func;/* associated SQLFunctionHashEntry */
145145

146+
boolactive;/* are we executing this cache entry? */
146147
boollazyEvalOK;/* true if lazyEval is safe */
147148
boolshutdown_reg;/* true if registered shutdown callback */
148149
boollazyEval;/* true if using lazyEval for result query */
@@ -556,6 +557,28 @@ init_sql_fcache(FunctionCallInfo fcinfo, bool lazyEvalOK)
556557
finfo->fn_extra=fcache;
557558
}
558559

560+
/*
561+
* If the SQLFunctionCache is marked as active, we must have errored out
562+
* of a prior execution. Reset state. (It might seem that we could also
563+
* reach this during recursive invocation of a SQL function, but we won't
564+
* because that case won't involve re-use of the same FmgrInfo.)
565+
*/
566+
if (fcache->active)
567+
{
568+
/*
569+
* In general, this stanza should clear all the same fields that
570+
* ShutdownSQLFunction would. Note we must clear fcache->cplan
571+
* without doing ReleaseCachedPlan, because error cleanup from the
572+
* prior execution would have taken care of releasing that plan.
573+
* Likewise, if tstore is still set then it is pointing at garbage.
574+
*/
575+
fcache->cplan=NULL;
576+
fcache->eslist=NULL;
577+
fcache->tstore=NULL;
578+
fcache->shutdown_reg= false;
579+
fcache->active= false;
580+
}
581+
559582
/*
560583
* If we are resuming execution of a set-returning function, just keep
561584
* using the same cache. We do not ask funccache.c to re-validate the
@@ -1597,6 +1620,9 @@ fmgr_sql(PG_FUNCTION_ARGS)
15971620
*/
15981621
fcache=init_sql_fcache(fcinfo,lazyEvalOK);
15991622

1623+
/* Mark fcache as active */
1624+
fcache->active= true;
1625+
16001626
/* Remember info that we might need later to construct tuplestore */
16011627
fcache->tscontext=tscontext;
16021628
fcache->randomAccess=randomAccess;
@@ -1853,6 +1879,9 @@ fmgr_sql(PG_FUNCTION_ARGS)
18531879
if (es==NULL)
18541880
fcache->eslist=NULL;
18551881

1882+
/* Mark fcache as inactive */
1883+
fcache->active= false;
1884+
18561885
error_context_stack=sqlerrcontext.previous;
18571886

18581887
returnresult;

‎src/test/regress/expected/create_function_sql.out‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,22 @@ SELECT double_append(array_append(ARRAY[q1], q2), q3)
733733
{4,5,6,4,5,6}
734734
(2 rows)
735735

736+
-- Check that we can re-use a SQLFunctionCache after a run-time error.
737+
-- This function will fail with zero-divide at run time (not plan time).
738+
CREATE FUNCTION part_hashint4_error(value int4, seed int8) RETURNS int8
739+
LANGUAGE SQL STRICT IMMUTABLE PARALLEL SAFE AS
740+
$$ SELECT value + seed + random()::int/0 $$;
741+
-- Put it into an operator class so that FmgrInfo will be cached in relcache.
742+
CREATE OPERATOR CLASS part_test_int4_ops_bad FOR TYPE int4 USING hash AS
743+
FUNCTION 2 part_hashint4_error(int4, int8);
744+
CREATE TABLE pt(i int) PARTITION BY hash (i part_test_int4_ops_bad);
745+
CREATE TABLE p1 PARTITION OF pt FOR VALUES WITH (modulus 4, remainder 0);
746+
INSERT INTO pt VALUES (1);
747+
ERROR: division by zero
748+
CONTEXT: SQL function "part_hashint4_error" statement 1
749+
INSERT INTO pt VALUES (1);
750+
ERROR: division by zero
751+
CONTEXT: SQL function "part_hashint4_error" statement 1
736752
-- Things that shouldn't work:
737753
CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
738754
AS 'SELECT ''not an integer'';';
@@ -773,7 +789,7 @@ CONTEXT: SQL function "test1" during startup
773789
RESET check_function_bodies;
774790
-- Cleanup
775791
DROP SCHEMA temp_func_test CASCADE;
776-
NOTICE: drop cascades to35 other objects
792+
NOTICE: drop cascades to38 other objects
777793
DETAIL: drop cascades to function functest_a_1(text,date)
778794
drop cascades to function functest_a_2(text[])
779795
drop cascades to function functest_a_3()
@@ -808,6 +824,9 @@ drop cascades to function create_and_insert()
808824
drop cascades to table ddl_test
809825
drop cascades to function alter_and_insert()
810826
drop cascades to function double_append(anyarray,anyelement)
827+
drop cascades to function part_hashint4_error(integer,bigint)
828+
drop cascades to operator family part_test_int4_ops_bad for access method hash
829+
drop cascades to table pt
811830
drop cascades to function test1(anyelement)
812831
DROP USER regress_unpriv_user;
813832
RESET search_path;

‎src/test/regress/sql/create_function_sql.sql‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,23 @@ $$ SELECT array_append($1, $2) || array_append($1, $2) $$;
432432
SELECT double_append(array_append(ARRAY[q1], q2), q3)
433433
FROM (VALUES(1,2,3), (4,5,6)) v(q1,q2,q3);
434434

435+
-- Check that we can re-use a SQLFunctionCache after a run-time error.
436+
437+
-- This function will fail with zero-divide at run time (not plan time).
438+
CREATEFUNCTIONpart_hashint4_error(value int4, seed int8) RETURNS int8
439+
LANGUAGE SQL STRICT IMMUTABLE PARALLEL SAFEAS
440+
$$SELECT value+ seed+ random()::int/0 $$;
441+
442+
-- Put it into an operator class so that FmgrInfo will be cached in relcache.
443+
CREATEOPERATOR CLASSpart_test_int4_ops_bad FOR TYPE int4 USING hashAS
444+
FUNCTION2 part_hashint4_error(int4, int8);
445+
446+
CREATETABLEpt(iint) PARTITION BY hash (i part_test_int4_ops_bad);
447+
CREATETABLEp1 PARTITION OF pt FORVALUES WITH (modulus4, remainder0);
448+
449+
INSERT INTO ptVALUES (1);
450+
INSERT INTO ptVALUES (1);
451+
435452
-- Things that shouldn't work:
436453

437454
CREATEFUNCTIONtest1 (int) RETURNSint LANGUAGE SQL

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp