]> granicus.if.org Git - postgresql/commitdiff
Make new expression eval code reject references to dropped columns.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 28 Mar 2017 22:05:03 +0000 (18:05 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 28 Mar 2017 22:05:14 +0000 (18:05 -0400)
Formerly, a Var referencing an already-dropped column was allowed and would
always produce a NULL value.  However, that behavior was implemented in
slot_getattr which the new expression code doesn't use; thus there is now a
risk of returning theoretically-deleted data.  We had regression test cases
that purported to exercise this, but they failed to expose any problem,
apparently because plpgsql filters the dropped column and produces an
output tuple that has a NULL there already.

Ideally the DROP or ALTER attempt in these test cases would get rejected
due to dependency checks; but until that happens, let's modify the behavior
so that we fail the query during executor start.  This was already true for
the related case of a column having changed type underneath us, and there's
no obvious reason why we need to be laxer for dropped columns.

In passing, adjust the error messages in CheckVarSlotCompatibility to
include the composite type name.  In the cases shown in the regression
tests this is always just "record", but it should be more useful in
actual stale-plan cases, where the slot tupdesc would be a table's
tupdesc directly.

Discussion: https://postgr.es/m/16803.1490723570@sss.pgh.pa.us

src/backend/executor/execExprInterp.c
src/test/regress/expected/create_view.out
src/test/regress/expected/rangefuncs.out
src/test/regress/sql/create_view.sql
src/test/regress/sql/rangefuncs.sql

index 982d16c6c88d8381d13bed0b98804f2190c5054e..4fbb5c1e74675cd42264231ae3e530e284916253 100644 (file)
@@ -1516,22 +1516,20 @@ CheckVarSlotCompatibility(TupleTableSlot *slot, int attnum, Oid vartype)
 {
        /*
         * What we have to check for here is the possibility of an attribute
-        * having been changed in type since the plan tree was created.  Ideally
-        * the plan will get invalidated and not re-used, but just in case, we
-        * keep these defenses.  Fortunately it's sufficient to check once on the
-        * first time through.
-        *
-        * System attributes don't require checking since their types never
-        * change.
-        *
-        * Note: we allow a reference to a dropped attribute.  slot_getattr will
-        * force a NULL result in such cases.
+        * having been dropped or changed in type since the plan tree was created.
+        * Ideally the plan will get invalidated and not re-used, but just in
+        * case, we keep these defenses.  Fortunately it's sufficient to check
+        * once on the first time through.
         *
         * Note: ideally we'd check typmod as well as typid, but that seems
         * impractical at the moment: in many cases the tupdesc will have been
         * generated by ExecTypeFromTL(), and that can't guarantee to generate an
         * accurate typmod in all cases, because some expression node types don't
-        * carry typmod.
+        * carry typmod.  Fortunately, for precisely that reason, there should be
+        * no places with a critical dependency on the typmod of a value.
+        *
+        * System attributes don't require checking since their types never
+        * change.
         */
        if (attnum > 0)
        {
@@ -1544,17 +1542,20 @@ CheckVarSlotCompatibility(TupleTableSlot *slot, int attnum, Oid vartype)
 
                attr = slot_tupdesc->attrs[attnum - 1];
 
-               /* can't check type if dropped, since atttypid is probably 0 */
-               if (!attr->attisdropped)
-               {
-                       if (vartype != attr->atttypid)
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_DATATYPE_MISMATCH),
-                                                errmsg("attribute %d has wrong type", attnum),
-                                                errdetail("Table has type %s, but query expects %s.",
-                                                                  format_type_be(attr->atttypid),
-                                                                  format_type_be(vartype))));
-               }
+               if (attr->attisdropped)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_UNDEFINED_COLUMN),
+                                        errmsg("attribute %d of type %s has been dropped",
+                                                       attnum, format_type_be(slot_tupdesc->tdtypeid))));
+
+               if (vartype != attr->atttypid)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_DATATYPE_MISMATCH),
+                                        errmsg("attribute %d of type %s has wrong type",
+                                                       attnum, format_type_be(slot_tupdesc->tdtypeid)),
+                                        errdetail("Table has type %s, but query expects %s.",
+                                                          format_type_be(attr->atttypid),
+                                                          format_type_be(vartype))));
        }
 }
 
index ce0c8cedf89f670cd18a4cedf1369dbdc8ecf123..c719262720eb4053a77917e0647add772ab425af 100644 (file)
@@ -1396,10 +1396,10 @@ select pg_get_viewdef('vv6', true);
 (1 row)
 
 --
--- Check some cases involving dropped columns in a function's rowtype result
+-- Check cases involving dropped/altered columns in a function's rowtype result
 --
 create table tt14t (f1 text, f2 text, f3 text, f4 text);
-insert into tt14t values('foo', 'bar', 'baz', 'quux');
+insert into tt14t values('foo', 'bar', 'baz', '42');
 alter table tt14t drop column f2;
 create function tt14f() returns setof tt14t as
 $$
@@ -1424,14 +1424,15 @@ select pg_get_viewdef('tt14v', true);
 (1 row)
 
 select * from tt14v;
- f1  | f3  |  f4  
------+-----+------
- foo | baz | quux
+ f1  | f3  | f4 
+-----+-----+----
+ foo | baz | 42
 (1 row)
 
+begin;
 -- this perhaps should be rejected, but it isn't:
 alter table tt14t drop column f3;
--- f3 is still in the view but will read as nulls
+-- f3 is still in the view ...
 select pg_get_viewdef('tt14v', true);
          pg_get_viewdef         
 --------------------------------
@@ -1441,12 +1442,40 @@ select pg_get_viewdef('tt14v', true);
     FROM tt14f() t(f1, f3, f4);
 (1 row)
 
+-- but will fail at execution
+select f1, f4 from tt14v;
+ f1  | f4 
+-----+----
+ foo | 42
+(1 row)
+
 select * from tt14v;
- f1  | f3 |  f4  
------+----+------
- foo |    | quux
+ERROR:  attribute 3 of type record has been dropped
+rollback;
+begin;
+-- this perhaps should be rejected, but it isn't:
+alter table tt14t alter column f4 type integer using f4::integer;
+-- f4 is still in the view ...
+select pg_get_viewdef('tt14v', true);
+         pg_get_viewdef         
+--------------------------------
+  SELECT t.f1,                 +
+     t.f3,                     +
+     t.f4                      +
+    FROM tt14f() t(f1, f3, f4);
 (1 row)
 
+-- but will fail at execution
+select f1, f3 from tt14v;
+ f1  | f3  
+-----+-----
+ foo | baz
+(1 row)
+
+select * from tt14v;
+ERROR:  attribute 4 of type record has wrong type
+DETAIL:  Table has type integer, but query expects text.
+rollback;
 -- check display of whole-row variables in some corner cases
 create type nestedcomposite as (x int8_tbl);
 create view tt15v as select row(i)::nestedcomposite from int8_tbl i;
index 526a4aed0a20c22384a2a1a9598daf308e46fc24..4b6170dea5abdf6807e5612dc681e99e859c115f 100644 (file)
@@ -1909,25 +1909,22 @@ select * from usersview;
  id2    |   2 | email2 |       12 | t       |              11 |          2
 (2 rows)
 
-alter table users drop column moredrop;
-select * from usersview;
- userid | seq | email  | moredrop | enabled | generate_series | ordinality 
---------+-----+--------+----------+---------+-----------------+------------
- id     |   1 | email  |          | t       |              10 |          1
- id2    |   2 | email2 |          | t       |              11 |          2
-(2 rows)
-
 alter table users add column junk text;
 select * from usersview;
  userid | seq | email  | moredrop | enabled | generate_series | ordinality 
 --------+-----+--------+----------+---------+-----------------+------------
- id     |   1 | email  |          | t       |              10 |          1
- id2    |   2 | email2 |          | t       |              11 |          2
+ id     |   1 | email  |       11 | t       |              10 |          1
+ id2    |   2 | email2 |       12 | t       |              11 |          2
 (2 rows)
 
+begin;
+alter table users drop column moredrop;
+select * from usersview;  -- expect clean failure
+ERROR:  attribute 5 of type record has been dropped
+rollback;
 alter table users alter column seq type numeric;
 select * from usersview;  -- expect clean failure
-ERROR:  attribute 2 has wrong type
+ERROR:  attribute 2 of type record has wrong type
 DETAIL:  Table has type numeric, but query expects integer.
 drop view usersview;
 drop function get_first_user();
index c27f1034e1384ac6341da943aa6dae2bb326c2ac..d6f50d6105f6c66ae83d352e40f97b3189cb5cd8 100644 (file)
@@ -457,11 +457,11 @@ alter table tt11 add column z int;
 select pg_get_viewdef('vv6', true);
 
 --
--- Check some cases involving dropped columns in a function's rowtype result
+-- Check cases involving dropped/altered columns in a function's rowtype result
 --
 
 create table tt14t (f1 text, f2 text, f3 text, f4 text);
-insert into tt14t values('foo', 'bar', 'baz', 'quux');
+insert into tt14t values('foo', 'bar', 'baz', '42');
 
 alter table tt14t drop column f2;
 
@@ -483,13 +483,32 @@ create view tt14v as select t.* from tt14f() t;
 select pg_get_viewdef('tt14v', true);
 select * from tt14v;
 
+begin;
+
 -- this perhaps should be rejected, but it isn't:
 alter table tt14t drop column f3;
 
--- f3 is still in the view but will read as nulls
+-- f3 is still in the view ...
 select pg_get_viewdef('tt14v', true);
+-- but will fail at execution
+select f1, f4 from tt14v;
 select * from tt14v;
 
+rollback;
+
+begin;
+
+-- this perhaps should be rejected, but it isn't:
+alter table tt14t alter column f4 type integer using f4::integer;
+
+-- f4 is still in the view ...
+select pg_get_viewdef('tt14v', true);
+-- but will fail at execution
+select f1, f3 from tt14v;
+select * from tt14v;
+
+rollback;
+
 -- check display of whole-row variables in some corner cases
 
 create type nestedcomposite as (x int8_tbl);
index 09ac8fbdb4df216521a0434b41105a1ca8da3ec3..4ed84b1f2b333b4954a59b102595a32204ab3e38 100644 (file)
@@ -555,11 +555,13 @@ SELECT * FROM ROWS FROM(get_users(), generate_series(10,11)) WITH ORDINALITY;
 create temp view usersview as
 SELECT * FROM ROWS FROM(get_users(), generate_series(10,11)) WITH ORDINALITY;
 
-select * from usersview;
-alter table users drop column moredrop;
 select * from usersview;
 alter table users add column junk text;
 select * from usersview;
+begin;
+alter table users drop column moredrop;
+select * from usersview;  -- expect clean failure
+rollback;
 alter table users alter column seq type numeric;
 select * from usersview;  -- expect clean failure