Skip to content

Commit 9257f07

Browse files
tglsfdctglsfdc
committed
Replace uses of SPI_modifytuple that intend to allocate in current context.
Invent a new function heap_modify_tuple_by_cols() that is functionally equivalent to SPI_modifytuple except that it always allocates its result by simple palloc. I chose however to make the API details a bit more like heap_modify_tuple: pass a tupdesc rather than a Relation, and use bool convention for the isnull array. Use this function in place of SPI_modifytuple at all call sites where the intended behavior is to allocate in current context. (There actually are only two call sites left that depend on the old behavior, which makes me wonder if we should just drop this function rather than keep it.) This new function is easier to use than heap_modify_tuple() for purposes of replacing a single column (or, really, any fixed number of columns). There are a number of places where it would simplify the code to change over, but I resisted that temptation for the moment ... everywhere except in plpgsql's exec_assign_value(); changing that might offer some small performance benefit, so I did it. This is on the way to removing SPI_push/SPI_pop, but it seems like good code cleanup in its own right. Discussion: <9633.1478552022@sss.pgh.pa.us>
1 parent dce429b commit 9257f07

File tree

10 files changed

+137
-95
lines changed

10 files changed

+137
-95
lines changed

‎contrib/spi/autoinc.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44
#include "postgres.h"
55

6+
#include "access/htup_details.h"
67
#include "catalog/pg_type.h"
78
#include "commands/sequence.h"
89
#include "commands/trigger.h"
@@ -23,6 +24,7 @@ autoinc(PG_FUNCTION_ARGS)
2324
int *chattrs; /* attnums of attributes to change */
2425
int chnattrs = 0; /* # of above */
2526
Datum *newvals; /* vals of above */
27+
bool *newnulls; /* null flags for above */
2628
char **args; /* arguments */
2729
char *relname; /* triggered relation name */
2830
Relation rel; /* triggered relation */
@@ -64,6 +66,7 @@ autoinc(PG_FUNCTION_ARGS)
6466

6567
chattrs = (int *) palloc(nargs / 2 * sizeof(int));
6668
newvals = (Datum *) palloc(nargs / 2 * sizeof(Datum));
69+
newnulls = (bool *) palloc(nargs / 2 * sizeof(bool));
6770

6871
for (i = 0; i < nargs;)
6972
{
@@ -102,23 +105,23 @@ autoinc(PG_FUNCTION_ARGS)
102105
newvals[chnattrs] = DirectFunctionCall1(nextval, seqname);
103106
newvals[chnattrs] = Int32GetDatum((int32) DatumGetInt64(newvals[chnattrs]));
104107
}
108+
newnulls[chnattrs] = false;
105109
pfree(DatumGetTextP(seqname));
106110
chnattrs++;
107111
i++;
108112
}
109113

110114
if (chnattrs > 0)
111115
{
112-
rettuple = SPI_modifytuple(rel, rettuple, chnattrs, chattrs, newvals, NULL);
113-
if (rettuple == NULL)
114-
/* internal error */
115-
elog(ERROR, "autoinc (%s): %d returned by SPI_modifytuple",
116-
relname, SPI_result);
116+
rettuple = heap_modify_tuple_by_cols(rettuple, tupdesc,
117+
chnattrs, chattrs,
118+
newvals, newnulls);
117119
}
118120

119121
pfree(relname);
120122
pfree(chattrs);
121123
pfree(newvals);
124+
pfree(newnulls);
122125

123126
return PointerGetDatum(rettuple);
124127
}

‎contrib/spi/insert_username.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
/*
2-
* insert_username.c
3-
* $Modified: Thu Oct 16 08:13:42 1997 by brook $
42
* contrib/spi/insert_username.c
53
*
64
* insert user name in response to a trigger
75
* usage: insert_username (column_name)
86
*/
97
#include "postgres.h"
108

9+
#include "access/htup_details.h"
1110
#include "catalog/pg_type.h"
1211
#include "commands/trigger.h"
1312
#include "executor/spi.h"
@@ -26,6 +25,7 @@ insert_username(PG_FUNCTION_ARGS)
2625
Trigger *trigger; /* to get trigger name */
2726
int nargs; /* # of arguments */
2827
Datum newval; /* new value of column */
28+
bool newnull; /* null flag */
2929
char **args; /* arguments */
3030
char *relname; /* triggered relation name */
3131
Relation rel; /* triggered relation */
@@ -80,13 +80,11 @@ insert_username(PG_FUNCTION_ARGS)
8080

8181
/* create fields containing name */
8282
newval = CStringGetTextDatum(GetUserNameFromId(GetUserId(), false));
83+
newnull = false;
8384

8485
/* construct new tuple */
85-
rettuple = SPI_modifytuple(rel, rettuple, 1, &attnum, &newval, NULL);
86-
if (rettuple == NULL)
87-
/* internal error */
88-
elog(ERROR, "insert_username (\"%s\"): %d returned by SPI_modifytuple",
89-
relname, SPI_result);
86+
rettuple = heap_modify_tuple_by_cols(rettuple, tupdesc,
87+
1, &attnum, &newval, &newnull);
9088

9189
pfree(relname);
9290

‎contrib/spi/moddatetime.c

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ OH, me, I'm Terry Mackintosh <terry@terrym.com>
1515
*/
1616
#include "postgres.h"
1717

18+
#include "access/htup_details.h"
1819
#include "catalog/pg_type.h"
1920
#include "executor/spi.h"
2021
#include "commands/trigger.h"
@@ -34,6 +35,7 @@ moddatetime(PG_FUNCTION_ARGS)
3435
int attnum; /* positional number of field to change */
3536
Oid atttypid; /* type OID of field to change */
3637
Datum newdt; /* The current datetime. */
38+
bool newdtnull; /* null flag for it */
3739
char **args; /* arguments */
3840
char *relname; /* triggered relation name */
3941
Relation rel; /* triggered relation */
@@ -115,22 +117,13 @@ moddatetime(PG_FUNCTION_ARGS)
115117
args[0], relname)));
116118
newdt = (Datum) 0; /* keep compiler quiet */
117119
}
120+
newdtnull = false;
118121

119-
/* 1 is the number of items in the arrays attnum and newdt.
120-
attnum is the positional number of the field to be updated.
121-
newdt is the new datetime stamp.
122-
NOTE that attnum and newdt are not arrays, but then a 1 element array
123-
is not an array any more then they are. Thus, they can be considered a
124-
one element array.
125-
*/
126-
rettuple = SPI_modifytuple(rel, rettuple, 1, &attnum, &newdt, NULL);
127-
128-
if (rettuple == NULL)
129-
/* internal error */
130-
elog(ERROR, "moddatetime (%s): %d returned by SPI_modifytuple",
131-
relname, SPI_result);
122+
/* Replace the attnum'th column with newdt */
123+
rettuple = heap_modify_tuple_by_cols(rettuple, tupdesc,
124+
1, &attnum, &newdt, &newdtnull);
132125

133-
/* Clean up */
126+
/* Clean up */
134127
pfree(relname);
135128

136129
return PointerGetDatum(rettuple);

‎contrib/spi/timetravel.c

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include <ctype.h>
1313

14+
#include "access/htup_details.h"
1415
#include "catalog/pg_type.h"
1516
#include "commands/trigger.h"
1617
#include "executor/spi.h"
@@ -183,13 +184,13 @@ timetravel(PG_FUNCTION_ARGS)
183184
int chnattrs = 0;
184185
int chattrs[MaxAttrNum];
185186
Datum newvals[MaxAttrNum];
186-
char newnulls[MaxAttrNum];
187+
bool newnulls[MaxAttrNum];
187188

188189
oldtimeon = SPI_getbinval(trigtuple, tupdesc, attnum[a_time_on], &isnull);
189190
if (isnull)
190191
{
191192
newvals[chnattrs] = GetCurrentAbsoluteTime();
192-
newnulls[chnattrs] = ' ';
193+
newnulls[chnattrs] = false;
193194
chattrs[chnattrs] = attnum[a_time_on];
194195
chnattrs++;
195196
}
@@ -201,7 +202,7 @@ timetravel(PG_FUNCTION_ARGS)
201202
(chnattrs > 0 && DatumGetInt32(newvals[a_time_on]) >= NOEND_ABSTIME))
202203
elog(ERROR, "timetravel (%s): %s is infinity", relname, args[a_time_on]);
203204
newvals[chnattrs] = NOEND_ABSTIME;
204-
newnulls[chnattrs] = ' ';
205+
newnulls[chnattrs] = false;
205206
chattrs[chnattrs] = attnum[a_time_off];
206207
chnattrs++;
207208
}
@@ -220,21 +221,23 @@ timetravel(PG_FUNCTION_ARGS)
220221
{
221222
/* clear update_user value */
222223
newvals[chnattrs] = nulltext;
223-
newnulls[chnattrs] = 'n';
224+
newnulls[chnattrs] = true;
224225
chattrs[chnattrs] = attnum[a_upd_user];
225226
chnattrs++;
226227
/* clear delete_user value */
227228
newvals[chnattrs] = nulltext;
228-
newnulls[chnattrs] = 'n';
229+
newnulls[chnattrs] = true;
229230
chattrs[chnattrs] = attnum[a_del_user];
230231
chnattrs++;
231232
/* set insert_user value */
232233
newvals[chnattrs] = newuser;
233-
newnulls[chnattrs] = ' ';
234+
newnulls[chnattrs] = false;
234235
chattrs[chnattrs] = attnum[a_ins_user];
235236
chnattrs++;
236237
}
237-
rettuple = SPI_modifytuple(rel, trigtuple, chnattrs, chattrs, newvals, newnulls);
238+
rettuple = heap_modify_tuple_by_cols(trigtuple, tupdesc,
239+
chnattrs, chattrs,
240+
newvals, newnulls);
238241
return PointerGetDatum(rettuple);
239242
/* end of INSERT */
240243
}
@@ -395,13 +398,11 @@ timetravel(PG_FUNCTION_ARGS)
395398
chnattrs++;
396399
}
397400

398-
rettuple = SPI_modifytuple(rel, newtuple, chnattrs, chattrs, newvals, newnulls);
399-
400401
/*
401-
* SPI_copytuple allocates tmptuple in upper executor context - have
402-
* to free allocation using SPI_pfree
402+
* Use SPI_modifytuple() here because we are inside SPI environment
403+
* but rettuple must be allocated in caller's context.
403404
*/
404-
/* SPI_pfree(tmptuple); */
405+
rettuple = SPI_modifytuple(rel, newtuple, chnattrs, chattrs, newvals, newnulls);
405406
}
406407
else
407408
/* DELETE case */

‎doc/src/sgml/spi.sgml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3382,8 +3382,9 @@ char * SPI_getnspname(Relation <parameter>rel</parameter>)
33823382
<function>repalloc</function>, or SPI utility functions (except for
33833383
<function>SPI_copytuple</function>,
33843384
<function>SPI_returntuple</function>,
3385-
<function>SPI_modifytuple</function>, and
3386-
<function>SPI_palloc</function>) are made in this context. When a
3385+
<function>SPI_modifytuple</function>,
3386+
<function>SPI_palloc</function>, and
3387+
<function>SPI_datumTransfer</function>) are made in this context. When a
33873388
procedure disconnects from the SPI manager (via
33883389
<function>SPI_finish</function>) the current context is restored to
33893390
the upper executor context, and all allocations made in the

‎src/backend/access/common/heaptuple.c

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,72 @@ heap_modify_tuple(HeapTuple tuple,
846846
return newTuple;
847847
}
848848

849+
/*
850+
* heap_modify_tuple_by_cols
851+
* form a new tuple from an old tuple and a set of replacement values.
852+
*
853+
* This is like heap_modify_tuple, except that instead of specifying which
854+
* column(s) to replace by a boolean map, an array of target column numbers
855+
* is used. This is often more convenient when a fixed number of columns
856+
* are to be replaced. The replCols, replValues, and replIsnull arrays must
857+
* be of length nCols. Target column numbers are indexed from 1.
858+
*
859+
* The result is allocated in the current memory context.
860+
*/
861+
HeapTuple
862+
heap_modify_tuple_by_cols(HeapTuple tuple,
863+
TupleDesc tupleDesc,
864+
int nCols,
865+
int *replCols,
866+
Datum *replValues,
867+
bool *replIsnull)
868+
{
869+
int numberOfAttributes = tupleDesc->natts;
870+
Datum *values;
871+
bool *isnull;
872+
HeapTuple newTuple;
873+
int i;
874+
875+
/*
876+
* allocate and fill values and isnull arrays from the tuple, then replace
877+
* selected columns from the input arrays.
878+
*/
879+
values = (Datum *) palloc(numberOfAttributes * sizeof(Datum));
880+
isnull = (bool *) palloc(numberOfAttributes * sizeof(bool));
881+
882+
heap_deform_tuple(tuple, tupleDesc, values, isnull);
883+
884+
for (i = 0; i < nCols; i++)
885+
{
886+
int attnum = replCols[i];
887+
888+
if (attnum <= 0 || attnum > numberOfAttributes)
889+
elog(ERROR, "invalid column number %d", attnum);
890+
values[attnum - 1] = replValues[i];
891+
isnull[attnum - 1] = replIsnull[i];
892+
}
893+
894+
/*
895+
* create a new tuple from the values and isnull arrays
896+
*/
897+
newTuple = heap_form_tuple(tupleDesc, values, isnull);
898+
899+
pfree(values);
900+
pfree(isnull);
901+
902+
/*
903+
* copy the identification info of the old tuple: t_ctid, t_self, and OID
904+
* (if any)
905+
*/
906+
newTuple->t_data->t_ctid = tuple->t_data->t_ctid;
907+
newTuple->t_self = tuple->t_self;
908+
newTuple->t_tableOid = tuple->t_tableOid;
909+
if (tupleDesc->tdhasoid)
910+
HeapTupleSetOid(newTuple, HeapTupleGetOid(tuple));
911+
912+
return newTuple;
913+
}
914+
849915
/*
850916
* heap_deform_tuple
851917
* Given a tuple, extract data into values/isnull arrays; this is

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2329,8 +2329,10 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column)
23292329
if (prs.curwords)
23302330
{
23312331
datum = PointerGetDatum(make_tsvector(&prs));
2332-
rettuple = SPI_modifytuple(rel, rettuple, 1, &tsvector_attr_num,
2333-
&datum, NULL);
2332+
isnull = false;
2333+
rettuple = heap_modify_tuple_by_cols(rettuple, rel->rd_att,
2334+
1, &tsvector_attr_num,
2335+
&datum, &isnull);
23342336
pfree(DatumGetPointer(datum));
23352337
}
23362338
else
@@ -2340,14 +2342,12 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column)
23402342
SET_VARSIZE(out, CALCDATASIZE(0, 0));
23412343
out->size = 0;
23422344
datum = PointerGetDatum(out);
2343-
rettuple = SPI_modifytuple(rel, rettuple, 1, &tsvector_attr_num,
2344-
&datum, NULL);
2345+
isnull = false;
2346+
rettuple = heap_modify_tuple_by_cols(rettuple, rel->rd_att,
2347+
1, &tsvector_attr_num,
2348+
&datum, &isnull);
23452349
pfree(prs.words);
23462350
}
23472351

2348-
if (rettuple == NULL) /* internal error */
2349-
elog(ERROR, "tsvector_update_trigger: %d returned by SPI_modifytuple",
2350-
SPI_result);
2351-
23522352
return PointerGetDatum(rettuple);
23532353
}

‎src/include/access/htup_details.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,12 @@ extern HeapTuple heap_modify_tuple(HeapTuple tuple,
805805
Datum *replValues,
806806
bool *replIsnull,
807807
bool *doReplace);
808+
extern HeapTuple heap_modify_tuple_by_cols(HeapTuple tuple,
809+
TupleDesc tupleDesc,
810+
int nCols,
811+
int *replCols,
812+
Datum *replValues,
813+
bool *replIsnull);
808814
extern void heap_deform_tuple(HeapTuple tuple, TupleDesc tupleDesc,
809815
Datum *values, bool *isnull);
810816
extern void heap_freetuple(HeapTuple htup);

0 commit comments

Comments
 (0)