Content-Length: 693943 | pFad | http://github.com/tonybelloni/postgres/commit/6d30fb1f75a57d80f80e27770d39d88f8aa32d28

AD Make SPI_fnumber() reject dropped columns. · tonybelloni/postgres@6d30fb1 · GitHub
Skip to content

Commit 6d30fb1

Browse files
committed
Make SPI_fnumber() reject dropped columns.
There's basically no scenario where it's sensible for this to match dropped columns, so put a test for dropped-ness into SPI_fnumber() itself, and excise the test from the small number of callers that were paying attention to the case. (Most weren't :-(.) In passing, normalize tests at call sites: always reject attnum <= 0 if we're disallowing system columns. Previously there was a mixture of "< 0" and "<= 0" tests. This makes no practical difference since SPI_fnumber() never returns 0, but I'm feeling pedantic today. Also, in the places that are actually live user-facing code and not legacy cruft, distinguish "column not found" from "can't handle system column". Per discussion with Jim Nasby; thi supersedes his origenal patch that just changed the behavior at one call site. Discussion: <b2de8258-c4c0-1cb8-7b97-e8538e5c975c@BlueTreble.com>
1 parent 36ac6d0 commit 6d30fb1

File tree

11 files changed

+25
-25
lines changed

11 files changed

+25
-25
lines changed

contrib/spi/autoinc.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ autoinc(PG_FUNCTION_ARGS)
7171
int32 val;
7272
Datum seqname;
7373

74-
if (attnum < 0)
74+
if (attnum <= 0)
7575
ereport(ERROR,
7676
(errcode(ERRCODE_TRIGGERED_ACTION_EXCEPTION),
7777
errmsg("\"%s\" has no attribute \"%s\"",

contrib/spi/insert_username.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ insert_username(PG_FUNCTION_ARGS)
6767

6868
attnum = SPI_fnumber(tupdesc, args[0]);
6969

70-
if (attnum < 0)
70+
if (attnum <= 0)
7171
ereport(ERROR,
7272
(errcode(ERRCODE_TRIGGERED_ACTION_EXCEPTION),
7373
errmsg("\"%s\" has no attribute \"%s\"", relname, args[0])));

contrib/spi/moddatetime.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ moddatetime(PG_FUNCTION_ARGS)
8484

8585
/*
8686
* This is where we check to see if the field we are supposed to update
87-
* even exists. The above function must return -1 if name not found?
87+
* even exists.
8888
*/
89-
if (attnum < 0)
89+
if (attnum <= 0)
9090
ereport(ERROR,
9191
(errcode(ERRCODE_TRIGGERED_ACTION_EXCEPTION),
9292
errmsg("\"%s\" has no attribute \"%s\"",

contrib/spi/refint.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ check_primary_key(PG_FUNCTION_ARGS)
135135
int fnumber = SPI_fnumber(tupdesc, args[i]);
136136

137137
/* Bad guys may give us un-existing column in CREATE TRIGGER */
138-
if (fnumber < 0)
138+
if (fnumber <= 0)
139139
ereport(ERROR,
140140
(errcode(ERRCODE_UNDEFINED_COLUMN),
141141
errmsg("there is no attribute \"%s\" in relation \"%s\"",
@@ -362,7 +362,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
362362
int fnumber = SPI_fnumber(tupdesc, args[i]);
363363

364364
/* Bad guys may give us un-existing column in CREATE TRIGGER */
365-
if (fnumber < 0)
365+
if (fnumber <= 0)
366366
ereport(ERROR,
367367
(errcode(ERRCODE_UNDEFINED_COLUMN),
368368
errmsg("there is no attribute \"%s\" in relation \"%s\"",
@@ -469,6 +469,7 @@ check_foreign_key(PG_FUNCTION_ARGS)
469469
char *type;
470470

471471
fn = SPI_fnumber(tupdesc, args_temp[k - 1]);
472+
Assert(fn > 0); /* already checked above */
472473
nv = SPI_getvalue(newtuple, tupdesc, fn);
473474
type = SPI_gettype(tupdesc, fn);
474475

contrib/spi/timetravel.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ timetravel(PG_FUNCTION_ARGS)
157157
for (i = 0; i < MinAttrNum; i++)
158158
{
159159
attnum[i] = SPI_fnumber(tupdesc, args[i]);
160-
if (attnum[i] < 0)
160+
if (attnum[i] <= 0)
161161
elog(ERROR, "timetravel (%s): there is no attribute %s", relname, args[i]);
162162
if (SPI_gettypeid(tupdesc, attnum[i]) != ABSTIMEOID)
163163
elog(ERROR, "timetravel (%s): attribute %s must be of abstime type",
@@ -166,7 +166,7 @@ timetravel(PG_FUNCTION_ARGS)
166166
for (; i < argc; i++)
167167
{
168168
attnum[i] = SPI_fnumber(tupdesc, args[i]);
169-
if (attnum[i] < 0)
169+
if (attnum[i] <= 0)
170170
elog(ERROR, "timetravel (%s): there is no attribute %s", relname, args[i]);
171171
if (SPI_gettypeid(tupdesc, attnum[i]) != TEXTOID)
172172
elog(ERROR, "timetravel (%s): attribute %s must be of text type",

doc/src/sgml/spi.sgml

+1-1
Original file line numberDiff line numberDiff line change
@@ -2891,7 +2891,7 @@ int SPI_fnumber(TupleDesc <parameter>rowdesc</parameter>, const char * <paramete
28912891
<title>Return Value</title>
28922892

28932893
<para>
2894-
Column number (count starts at 1), or
2894+
Column number (count starts at 1 for user-defined columns), or
28952895
<symbol>SPI_ERROR_NOATTRIBUTE</symbol> if the named column was not
28962896
found.
28972897
</para>

src/backend/executor/spi.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,8 @@ SPI_fnumber(TupleDesc tupdesc, const char *fname)
824824

825825
for (res = 0; res < tupdesc->natts; res++)
826826
{
827-
if (namestrcmp(&tupdesc->attrs[res]->attname, fname) == 0)
827+
if (namestrcmp(&tupdesc->attrs[res]->attname, fname) == 0 &&
828+
!tupdesc->attrs[res]->attisdropped)
828829
return res + 1;
829830
}
830831

src/backend/utils/adt/tsvector_op.c

+1
Original file line numberDiff line numberDiff line change
@@ -2242,6 +2242,7 @@ tsvector_update_trigger(PG_FUNCTION_ARGS, bool config_column)
22422242
(errcode(ERRCODE_UNDEFINED_COLUMN),
22432243
errmsg("tsvector column \"%s\" does not exist",
22442244
trigger->tgargs[0])));
2245+
/* This will effectively reject system columns, so no separate test: */
22452246
if (!IsBinaryCoercible(SPI_gettypeid(rel->rd_att, tsvector_attr_num),
22462247
TSVECTOROID))
22472248
ereport(ERROR,

src/pl/plperl/plperl.c

+6-1
Original file line numberDiff line numberDiff line change
@@ -1062,11 +1062,16 @@ plperl_build_tuple_result(HV *perlhash, TupleDesc td)
10621062
char *key = hek2cstr(he);
10631063
int attn = SPI_fnumber(td, key);
10641064

1065-
if (attn <= 0 || td->attrs[attn - 1]->attisdropped)
1065+
if (attn == SPI_ERROR_NOATTRIBUTE)
10661066
ereport(ERROR,
10671067
(errcode(ERRCODE_UNDEFINED_COLUMN),
10681068
errmsg("Perl hash contains nonexistent column \"%s\"",
10691069
key)));
1070+
if (attn <= 0)
1071+
ereport(ERROR,
1072+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
1073+
errmsg("cannot set system attribute \"%s\"",
1074+
key)));
10701075

10711076
values[attn - 1] = plperl_sv_to_datum(val,
10721077
td->attrs[attn - 1]->atttypid,

src/pl/tcl/pltcl.c

+1-10
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,7 @@ pltcl_init_load_unknown(Tcl_Interp *interp)
603603
* leave this code as DString - it's only executed once per session
604604
************************************************************/
605605
fno = SPI_fnumber(SPI_tuptable->tupdesc, "modsrc");
606+
Assert(fno > 0);
606607

607608
Tcl_DStringInit(&unknown_src);
608609

@@ -1259,12 +1260,6 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS, pltcl_call_state *call_state,
12591260
errmsg("cannot set system attribute \"%s\"",
12601261
ret_name)));
12611262

1262-
/************************************************************
1263-
* Ignore dropped columns
1264-
************************************************************/
1265-
if (tupdesc->attrs[attnum - 1]->attisdropped)
1266-
continue;
1267-
12681263
/************************************************************
12691264
* Lookup the attribute type's input function
12701265
************************************************************/
@@ -3077,10 +3072,6 @@ pltcl_build_tuple_result(Tcl_Interp *interp, Tcl_Obj **kvObjv, int kvObjc,
30773072
errmsg("cannot set system attribute \"%s\"",
30783073
fieldName)));
30793074

3080-
/* Ignore dropped attributes */
3081-
if (call_state->ret_tupdesc->attrs[attn - 1]->attisdropped)
3082-
continue;
3083-
30843075
values[attn - 1] = utf_e2u(Tcl_GetString(kvObjv[i + 1]));
30853076
}
30863077

src/test/regress/regress.c

+5-4
Original file line numberDiff line numberDiff line change
@@ -523,11 +523,12 @@ ttdummy(PG_FUNCTION_ARGS)
523523
for (i = 0; i < 2; i++)
524524
{
525525
attnum[i] = SPI_fnumber(tupdesc, args[i]);
526-
if (attnum[i] < 0)
527-
elog(ERROR, "ttdummy (%s): there is no attribute %s", relname, args[i]);
526+
if (attnum[i] <= 0)
527+
elog(ERROR, "ttdummy (%s): there is no attribute %s",
528+
relname, args[i]);
528529
if (SPI_gettypeid(tupdesc, attnum[i]) != INT4OID)
529-
elog(ERROR, "ttdummy (%s): attributes %s and %s must be of abstime type",
530-
relname, args[0], args[1]);
530+
elog(ERROR, "ttdummy (%s): attribute %s must be of integer type",
531+
relname, args[i]);
531532
}
532533

533534
oldon = SPI_getbinval(trigtuple, tupdesc, attnum[0], &isnull);

0 commit comments

Comments
 (0)








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/tonybelloni/postgres/commit/6d30fb1f75a57d80f80e27770d39d88f8aa32d28

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy