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

Commit78d5952

Browse files
committed
Ensure result of an aggregate's finalfunc is made read-only.
The finalfunc might return a read-write expanded object. If wede-duplicate multiple call sites for the aggregate, any function(s)receiving the aggregate result earlier could alter or destroy thevalue that reaches the ones called later. This is a brown-paper-bagbug in commit42b746d, because we actually considered the needfor read-only-ness but failed to realize that it applied to the casewith a finalfunc as well as the case without.Per report from Justin Pryzby. New error in HEAD,no need for back-patch.Discussion:https://postgr.es/m/ZDm5TuKsh3tzoEjz@telsasoft.com
1 parent1c54b93 commit78d5952

File tree

4 files changed

+96
-5
lines changed

4 files changed

+96
-5
lines changed

‎src/backend/executor/nodeAgg.c

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,9 +1040,10 @@ process_ordered_aggregate_multi(AggState *aggstate,
10401040
* (But note that in some cases, such as when there is no finalfn, the
10411041
* result might be a pointer to or into the agg's transition value.)
10421042
*
1043-
* The finalfn uses the state as set in the transno. This also might be
1043+
* The finalfn uses the state as set in the transno.This also might be
10441044
* being used by another aggregate function, so it's important that we do
1045-
* nothing destructive here.
1045+
* nothing destructive here. Moreover, the aggregate's final value might
1046+
* get used in multiple places, so we mustn't return a R/W expanded datum.
10461047
*/
10471048
staticvoid
10481049
finalize_aggregate(AggState*aggstate,
@@ -1116,8 +1117,13 @@ finalize_aggregate(AggState *aggstate,
11161117
}
11171118
else
11181119
{
1119-
*resultVal=FunctionCallInvoke(fcinfo);
1120+
Datumresult;
1121+
1122+
result=FunctionCallInvoke(fcinfo);
11201123
*resultIsNull=fcinfo->isnull;
1124+
*resultVal=MakeExpandedObjectReadOnly(result,
1125+
fcinfo->isnull,
1126+
peragg->resulttypeLen);
11211127
}
11221128
aggstate->curperagg=NULL;
11231129
}
@@ -1165,6 +1171,7 @@ finalize_partialaggregate(AggState *aggstate,
11651171
else
11661172
{
11671173
FunctionCallInfofcinfo=pertrans->serialfn_fcinfo;
1174+
Datumresult;
11681175

11691176
fcinfo->args[0].value=
11701177
MakeExpandedObjectReadOnly(pergroupstate->transValue,
@@ -1173,8 +1180,11 @@ finalize_partialaggregate(AggState *aggstate,
11731180
fcinfo->args[0].isnull=pergroupstate->transValueIsNull;
11741181
fcinfo->isnull= false;
11751182

1176-
*resultVal=FunctionCallInvoke(fcinfo);
1183+
result=FunctionCallInvoke(fcinfo);
11771184
*resultIsNull=fcinfo->isnull;
1185+
*resultVal=MakeExpandedObjectReadOnly(result,
1186+
fcinfo->isnull,
1187+
peragg->resulttypeLen);
11781188
}
11791189
}
11801190
else

‎src/backend/executor/nodeWindowAgg.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,10 +623,15 @@ finalize_windowaggregate(WindowAggState *winstate,
623623
}
624624
else
625625
{
626+
Datumres;
627+
626628
winstate->curaggcontext=peraggstate->aggcontext;
627-
*result=FunctionCallInvoke(fcinfo);
629+
res=FunctionCallInvoke(fcinfo);
628630
winstate->curaggcontext=NULL;
629631
*isnull=fcinfo->isnull;
632+
*result=MakeExpandedObjectReadOnly(res,
633+
fcinfo->isnull,
634+
peraggstate->resulttypeLen);
630635
}
631636
}
632637
else

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2758,6 +2758,43 @@ SELECT balk(hundred) FROM tenk1;
27582758

27592759
(1 row)
27602760

2761+
ROLLBACK;
2762+
-- test multiple usage of an aggregate whose finalfn returns a R/W datum
2763+
BEGIN;
2764+
CREATE FUNCTION rwagg_sfunc(x anyarray, y anyarray) RETURNS anyarray
2765+
LANGUAGE plpgsql IMMUTABLE AS $$
2766+
BEGIN
2767+
RETURN array_fill(y[1], ARRAY[4]);
2768+
END;
2769+
$$;
2770+
CREATE FUNCTION rwagg_finalfunc(x anyarray) RETURNS anyarray
2771+
LANGUAGE plpgsql STRICT IMMUTABLE AS $$
2772+
DECLARE
2773+
res x%TYPE;
2774+
BEGIN
2775+
-- assignment is essential for this test, it expands the array to R/W
2776+
res := array_fill(x[1], ARRAY[4]);
2777+
RETURN res;
2778+
END;
2779+
$$;
2780+
CREATE AGGREGATE rwagg(anyarray) (
2781+
STYPE = anyarray,
2782+
SFUNC = rwagg_sfunc,
2783+
FINALFUNC = rwagg_finalfunc
2784+
);
2785+
CREATE FUNCTION eatarray(x real[]) RETURNS real[]
2786+
LANGUAGE plpgsql STRICT IMMUTABLE AS $$
2787+
BEGIN
2788+
x[1] := x[1] + 1;
2789+
RETURN x;
2790+
END;
2791+
$$;
2792+
SELECT eatarray(rwagg(ARRAY[1.0::real])), eatarray(rwagg(ARRAY[1.0::real]));
2793+
eatarray | eatarray
2794+
-----------+-----------
2795+
{2,1,1,1} | {2,1,1,1}
2796+
(1 row)
2797+
27612798
ROLLBACK;
27622799
-- test coverage for aggregate combine/serial/deserial functions
27632800
BEGIN;

‎src/test/regress/sql/aggregates.sql

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,45 @@ SELECT balk(hundred) FROM tenk1;
12071207

12081208
ROLLBACK;
12091209

1210+
-- test multiple usage of an aggregate whose finalfn returns a R/W datum
1211+
BEGIN;
1212+
1213+
CREATEFUNCTIONrwagg_sfunc(x anyarray, y anyarray) RETURNS anyarray
1214+
LANGUAGE plpgsql IMMUTABLEAS $$
1215+
BEGIN
1216+
RETURN array_fill(y[1], ARRAY[4]);
1217+
END;
1218+
$$;
1219+
1220+
CREATEFUNCTIONrwagg_finalfunc(x anyarray) RETURNS anyarray
1221+
LANGUAGE plpgsql STRICT IMMUTABLEAS $$
1222+
DECLARE
1223+
res x%TYPE;
1224+
BEGIN
1225+
-- assignment is essential for this test, it expands the array to R/W
1226+
res := array_fill(x[1], ARRAY[4]);
1227+
RETURN res;
1228+
END;
1229+
$$;
1230+
1231+
CREATEAGGREGATErwagg(anyarray) (
1232+
STYPE= anyarray,
1233+
SFUNC= rwagg_sfunc,
1234+
FINALFUNC= rwagg_finalfunc
1235+
);
1236+
1237+
CREATEFUNCTIONeatarray(xreal[]) RETURNSreal[]
1238+
LANGUAGE plpgsql STRICT IMMUTABLEAS $$
1239+
BEGIN
1240+
x[1] := x[1]+1;
1241+
RETURN x;
1242+
END;
1243+
$$;
1244+
1245+
SELECT eatarray(rwagg(ARRAY[1.0::real])), eatarray(rwagg(ARRAY[1.0::real]));
1246+
1247+
ROLLBACK;
1248+
12101249
-- test coverage for aggregate combine/serial/deserial functions
12111250
BEGIN;
12121251

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp