Skip to content

Commit 2602ee4

Browse files
tglsfdctglsfdc
committed
Don't crash on reference to an un-available system column.
Adopt a more consistent policy about what slot-type-specific getsysattr functions should do when system attributes are not available. To wit, they should all throw the same user-oriented error, rather than variously crashing or emitting developer-oriented messages. This closes a identifiable problem in commits a71cfc5 and 3fb9310 (in v13 and v12), so back-patch into those branches, along with a test case to try to ensure we don't break it again. It is not known that any of the former crash cases are reachable in HEAD, but this seems like a good safety improvement in any case. Discussion: https://postgr.es/m/141051591267657@mail.yandex.ru
1 parent 00037d8 commit 2602ee4

File tree

3 files changed

+120
-5
lines changed

3 files changed

+120
-5
lines changed

‎src/backend/executor/execTuples.c

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,20 +122,28 @@ tts_virtual_clear(TupleTableSlot *slot)
122122
}
123123

124124
/*
125-
* Attribute values are readily available in tts_values and tts_isnull array
126-
* in a VirtualTupleTableSlot. So there should be no need to call either of the
127-
* following two functions.
125+
* VirtualTupleTableSlots always have fully populated tts_values and
126+
* tts_isnull arrays. So this function should never be called.
128127
*/
129128
static void
130129
tts_virtual_getsomeattrs(TupleTableSlot *slot, int natts)
131130
{
132131
elog(ERROR, "getsomeattrs is not required to be called on a virtual tuple table slot");
133132
}
134133

134+
/*
135+
* VirtualTupleTableSlots never provide system attributes (except those
136+
* handled generically, such as tableoid). We generally shouldn't get
137+
* here, but provide a user-friendly message if we do.
138+
*/
135139
static Datum
136140
tts_virtual_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull)
137141
{
138-
elog(ERROR, "virtual tuple table slot does not have system attributes");
142+
Assert(!TTS_EMPTY(slot));
143+
144+
ereport(ERROR,
145+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
146+
errmsg("cannot retrieve a system column in this context")));
139147

140148
return 0; /* silence compiler warnings */
141149
}
@@ -335,6 +343,15 @@ tts_heap_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull)
335343

336344
Assert(!TTS_EMPTY(slot));
337345

346+
/*
347+
* In some code paths it's possible to get here with a non-materialized
348+
* slot, in which case we can't retrieve system columns.
349+
*/
350+
if (!hslot->tuple)
351+
ereport(ERROR,
352+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
353+
errmsg("cannot retrieve a system column in this context")));
354+
338355
return heap_getsysattr(hslot->tuple, attnum,
339356
slot->tts_tupleDescriptor, isnull);
340357
}
@@ -497,7 +514,11 @@ tts_minimal_getsomeattrs(TupleTableSlot *slot, int natts)
497514
static Datum
498515
tts_minimal_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull)
499516
{
500-
elog(ERROR, "minimal tuple table slot does not have system attributes");
517+
Assert(!TTS_EMPTY(slot));
518+
519+
ereport(ERROR,
520+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
521+
errmsg("cannot retrieve a system column in this context")));
501522

502523
return 0; /* silence compiler warnings */
503524
}
@@ -681,6 +702,15 @@ tts_buffer_heap_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull)
681702

682703
Assert(!TTS_EMPTY(slot));
683704

705+
/*
706+
* In some code paths it's possible to get here with a non-materialized
707+
* slot, in which case we can't retrieve system columns.
708+
*/
709+
if (!bslot->base.tuple)
710+
ereport(ERROR,
711+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
712+
errmsg("cannot retrieve a system column in this context")));
713+
684714
return heap_getsysattr(bslot->base.tuple, attnum,
685715
slot->tts_tupleDescriptor, isnull);
686716
}

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,59 @@ DETAIL: Failing row contains (a, 10).
834834
-- ok
835835
UPDATE list_default set a = 'x' WHERE a = 'd';
836836
DROP TABLE list_parted;
837+
-- Test retrieval of system columns with non-consistent partition row types.
838+
-- This is only partially supported, as seen in the results.
839+
create table utrtest (a int, b text) partition by list (a);
840+
create table utr1 (a int check (a in (1)), q text, b text);
841+
create table utr2 (a int check (a in (2)), b text);
842+
alter table utr1 drop column q;
843+
alter table utrtest attach partition utr1 for values in (1);
844+
alter table utrtest attach partition utr2 for values in (2);
845+
insert into utrtest values (1, 'foo')
846+
returning *, tableoid::regclass, xmin = pg_current_xact_id()::xid as xmin_ok;
847+
a | b | tableoid | xmin_ok
848+
---+-----+----------+---------
849+
1 | foo | utr1 | t
850+
(1 row)
851+
852+
insert into utrtest values (2, 'bar')
853+
returning *, tableoid::regclass, xmin = pg_current_xact_id()::xid as xmin_ok; -- fails
854+
ERROR: cannot retrieve a system column in this context
855+
insert into utrtest values (2, 'bar')
856+
returning *, tableoid::regclass;
857+
a | b | tableoid
858+
---+-----+----------
859+
2 | bar | utr2
860+
(1 row)
861+
862+
update utrtest set b = b || b from (values (1), (2)) s(x) where a = s.x
863+
returning *, tableoid::regclass, xmin = pg_current_xact_id()::xid as xmin_ok;
864+
a | b | x | tableoid | xmin_ok
865+
---+--------+---+----------+---------
866+
1 | foofoo | 1 | utr1 | t
867+
2 | barbar | 2 | utr2 | t
868+
(2 rows)
869+
870+
update utrtest set a = 3 - a from (values (1), (2)) s(x) where a = s.x
871+
returning *, tableoid::regclass, xmin = pg_current_xact_id()::xid as xmin_ok; -- fails
872+
ERROR: cannot retrieve a system column in this context
873+
update utrtest set a = 3 - a from (values (1), (2)) s(x) where a = s.x
874+
returning *, tableoid::regclass;
875+
a | b | x | tableoid
876+
---+--------+---+----------
877+
2 | foofoo | 1 | utr2
878+
1 | barbar | 2 | utr1
879+
(2 rows)
880+
881+
delete from utrtest
882+
returning *, tableoid::regclass, xmax = pg_current_xact_id()::xid as xmax_ok;
883+
a | b | tableoid | xmax_ok
884+
---+--------+----------+---------
885+
1 | barbar | utr1 | t
886+
2 | foofoo | utr2 | t
887+
(2 rows)
888+
889+
drop table utrtest;
837890
--------------
838891
-- Some more update-partition-key test scenarios below. This time use list
839892
-- partitions.

‎src/test/regress/sql/update.sql

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,38 @@ UPDATE list_default set a = 'x' WHERE a = 'd';
527527

528528
DROP TABLE list_parted;
529529

530+
-- Test retrieval of system columns with non-consistent partition row types.
531+
-- This is only partially supported, as seen in the results.
532+
533+
create table utrtest (a int, b text) partition by list (a);
534+
create table utr1 (a int check (a in (1)), q text, b text);
535+
create table utr2 (a int check (a in (2)), b text);
536+
alter table utr1 drop column q;
537+
alter table utrtest attach partition utr1 for values in (1);
538+
alter table utrtest attach partition utr2 for values in (2);
539+
540+
insert into utrtest values (1, 'foo')
541+
returning *, tableoid::regclass, xmin = pg_current_xact_id()::xid as xmin_ok;
542+
insert into utrtest values (2, 'bar')
543+
returning *, tableoid::regclass, xmin = pg_current_xact_id()::xid as xmin_ok; -- fails
544+
insert into utrtest values (2, 'bar')
545+
returning *, tableoid::regclass;
546+
547+
update utrtest set b = b || b from (values (1), (2)) s(x) where a = s.x
548+
returning *, tableoid::regclass, xmin = pg_current_xact_id()::xid as xmin_ok;
549+
550+
update utrtest set a = 3 - a from (values (1), (2)) s(x) where a = s.x
551+
returning *, tableoid::regclass, xmin = pg_current_xact_id()::xid as xmin_ok; -- fails
552+
553+
update utrtest set a = 3 - a from (values (1), (2)) s(x) where a = s.x
554+
returning *, tableoid::regclass;
555+
556+
delete from utrtest
557+
returning *, tableoid::regclass, xmax = pg_current_xact_id()::xid as xmax_ok;
558+
559+
drop table utrtest;
560+
561+
530562
--------------
531563
-- Some more update-partition-key test scenarios below. This time use list
532564
-- partitions.

0 commit comments

Comments
 (0)