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

Commit5836d32

Browse files
committed
Fix minor violations of FunctionCallInvoke usage protocol.
Working on commit1c45507 led me to check through FunctionCallInvokecall sites to see if every one was being honest about (a) making surethat fcinfo.isnull is initially false, and (b) checking its state afterthe call. Sure enough, I found some violations.The main one is that finalize_partialaggregate re-used serialfn_fcinfowithout resetting isnull, even though it clearly intends to cater forserialfns that return NULL. There would only be an issue with anon-strict serialfn, since it's unlikely that a serialfn would returnNULL for non-null input. We have no non-strict serialfns in core, andthere may be none in the wild either, which would account for the lackof complaints. Still, it's clearly wrong, so back-patch that fix to9.6 where finalize_partialaggregate was introduced.Also, arrayfuncs.c and rowtypes.c contained various callers that werenot bothering to check for result nulls. While what's being called isa comparison or hash function that probably *shouldn't* return null,that's a lousy excuse for not having any check at all. There areexisting places that just Assert(!fcinfo->isnull) in comparablesituations, so I added that to the places that were calling btreecomparison or hash support functions. In the places callingboolean-returning equality functions, it's quite cheap to have themtreat isnull as FALSE, so make those places do that. Also remove some"locfcinfo->isnull = false" assignments that are unnecessary given theassumption that no previous call returned null. These changes seem likemostly neatnik-ism or debugging support, so I didn't back-patch.
1 parentafccd76 commit5836d32

File tree

4 files changed

+29
-12
lines changed

4 files changed

+29
-12
lines changed

‎src/backend/executor/nodeAgg.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,6 +1172,7 @@ finalize_partialaggregate(AggState *aggstate,
11721172
pergroupstate->transValueIsNull,
11731173
pertrans->transtypeLen);
11741174
fcinfo->args[0].isnull=pergroupstate->transValueIsNull;
1175+
fcinfo->isnull= false;
11751176

11761177
*resultVal=FunctionCallInvoke(fcinfo);
11771178
*resultIsNull=fcinfo->isnull;

‎src/backend/utils/adt/arrayfuncs.c

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3668,15 +3668,15 @@ array_eq(PG_FUNCTION_ARGS)
36683668
}
36693669

36703670
/*
3671-
* Apply the operator to the element pair
3671+
* Apply the operator to the element pair; treat NULL as false
36723672
*/
36733673
locfcinfo->args[0].value=elt1;
36743674
locfcinfo->args[0].isnull= false;
36753675
locfcinfo->args[1].value=elt2;
36763676
locfcinfo->args[1].isnull= false;
36773677
locfcinfo->isnull= false;
36783678
oprresult=DatumGetBool(FunctionCallInvoke(locfcinfo));
3679-
if (!oprresult)
3679+
if (locfcinfo->isnull||!oprresult)
36803680
{
36813681
result= false;
36823682
break;
@@ -3841,9 +3841,11 @@ array_cmp(FunctionCallInfo fcinfo)
38413841
locfcinfo->args[0].isnull= false;
38423842
locfcinfo->args[1].value=elt2;
38433843
locfcinfo->args[1].isnull= false;
3844-
locfcinfo->isnull= false;
38453844
cmpresult=DatumGetInt32(FunctionCallInvoke(locfcinfo));
38463845

3846+
/* We don't expect comparison support functions to return null */
3847+
Assert(!locfcinfo->isnull);
3848+
38473849
if (cmpresult==0)
38483850
continue;/* equal */
38493851

@@ -3983,8 +3985,9 @@ hash_array(PG_FUNCTION_ARGS)
39833985
/* Apply the hash function */
39843986
locfcinfo->args[0].value=elt;
39853987
locfcinfo->args[0].isnull= false;
3986-
locfcinfo->isnull= false;
39873988
elthash=DatumGetUInt32(FunctionCallInvoke(locfcinfo));
3989+
/* We don't expect hash functions to return null */
3990+
Assert(!locfcinfo->isnull);
39883991
}
39893992

39903993
/*
@@ -4074,6 +4077,8 @@ hash_array_extended(PG_FUNCTION_ARGS)
40744077
locfcinfo->args[1].value=Int64GetDatum(seed);
40754078
locfcinfo->args[1].isnull= false;
40764079
elthash=DatumGetUInt64(FunctionCallInvoke(locfcinfo));
4080+
/* We don't expect hash functions to return null */
4081+
Assert(!locfcinfo->isnull);
40774082
}
40784083

40794084
result= (result <<5)-result+elthash;
@@ -4207,15 +4212,15 @@ array_contain_compare(AnyArrayType *array1, AnyArrayType *array2, Oid collation,
42074212
continue;/* can't match */
42084213

42094214
/*
4210-
* Apply the operator to the element pair
4215+
* Apply the operator to the element pair; treat NULL as false
42114216
*/
42124217
locfcinfo->args[0].value=elt1;
42134218
locfcinfo->args[0].isnull= false;
42144219
locfcinfo->args[1].value=elt2;
42154220
locfcinfo->args[1].isnull= false;
42164221
locfcinfo->isnull= false;
42174222
oprresult=DatumGetBool(FunctionCallInvoke(locfcinfo));
4218-
if (oprresult)
4223+
if (!locfcinfo->isnull&&oprresult)
42194224
break;
42204225
}
42214226

@@ -6202,15 +6207,15 @@ array_replace_internal(ArrayType *array,
62026207
else
62036208
{
62046209
/*
6205-
* Apply the operator to the element pair
6210+
* Apply the operator to the element pair; treat NULL as false
62066211
*/
62076212
locfcinfo->args[0].value=elt;
62086213
locfcinfo->args[0].isnull= false;
62096214
locfcinfo->args[1].value=search;
62106215
locfcinfo->args[1].isnull= false;
62116216
locfcinfo->isnull= false;
62126217
oprresult=DatumGetBool(FunctionCallInvoke(locfcinfo));
6213-
if (!oprresult)
6218+
if (locfcinfo->isnull||!oprresult)
62146219
{
62156220
/* no match, keep element */
62166221
values[nresult]=elt;
@@ -6517,10 +6522,12 @@ width_bucket_array_fixed(Datum operand,
65176522
locfcinfo->args[0].isnull= false;
65186523
locfcinfo->args[1].value=fetch_att(ptr,typbyval,typlen);
65196524
locfcinfo->args[1].isnull= false;
6520-
locfcinfo->isnull= false;
65216525

65226526
cmpresult=DatumGetInt32(FunctionCallInvoke(locfcinfo));
65236527

6528+
/* We don't expect comparison support functions to return null */
6529+
Assert(!locfcinfo->isnull);
6530+
65246531
if (cmpresult<0)
65256532
right=mid;
65266533
else
@@ -6577,6 +6584,9 @@ width_bucket_array_variable(Datum operand,
65776584

65786585
cmpresult=DatumGetInt32(FunctionCallInvoke(locfcinfo));
65796586

6587+
/* We don't expect comparison support functions to return null */
6588+
Assert(!locfcinfo->isnull);
6589+
65806590
if (cmpresult<0)
65816591
right=mid;
65826592
else

‎src/backend/utils/adt/rowtypes.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -966,9 +966,11 @@ record_cmp(FunctionCallInfo fcinfo)
966966
locfcinfo->args[0].isnull= false;
967967
locfcinfo->args[1].value=values2[i2];
968968
locfcinfo->args[1].isnull= false;
969-
locfcinfo->isnull= false;
970969
cmpresult=DatumGetInt32(FunctionCallInvoke(locfcinfo));
971970

971+
/* We don't expect comparison support functions to return null */
972+
Assert(!locfcinfo->isnull);
973+
972974
if (cmpresult<0)
973975
{
974976
/* arg1 is less than arg2 */
@@ -1200,9 +1202,8 @@ record_eq(PG_FUNCTION_ARGS)
12001202
locfcinfo->args[0].isnull= false;
12011203
locfcinfo->args[1].value=values2[i2];
12021204
locfcinfo->args[1].isnull= false;
1203-
locfcinfo->isnull= false;
12041205
oprresult=DatumGetBool(FunctionCallInvoke(locfcinfo));
1205-
if (!oprresult)
1206+
if (locfcinfo->isnull||!oprresult)
12061207
{
12071208
result= false;
12081209
break;

‎src/include/fmgr.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,11 @@ extern void fmgr_symbol(Oid functionId, char **mod, char **fn);
163163
* caller must still check fcinfo->isnull!Also, if function is strict,
164164
* it is caller's responsibility to verify that no null arguments are present
165165
* before calling.
166+
*
167+
* Some code performs multiple calls without redoing InitFunctionCallInfoData,
168+
* possibly altering the argument values. This is okay, but be sure to reset
169+
* the fcinfo->isnull flag before each call, since callees are permitted to
170+
* assume that starts out false.
166171
*/
167172
#defineFunctionCallInvoke(fcinfo)((* (fcinfo)->flinfo->fn_addr) (fcinfo))
168173

0 commit comments

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