Rethink the dependencies recorded for FieldSelect/FieldStore nodes.

On closer investigation, commits f3ea3e3e8 et al were a few bricks
shy of a load.  What we need is not so much to lock down the result
type of a FieldSelect, as to lock down the existence of the column
it's trying to extract.  Otherwise, we can break it by dropping that
column.  The dependency on the result type is then held indirectly
through the column, and doesn't need to be recorded explicitly.

Out of paranoia, I left in the code to record a dependency on the
result type, but it's used only if we can't identify the pg_class OID
for the column.  That shouldn't ever happen right now, AFAICS, but
it seems possible that in future the input node could be marked as
being of type RECORD rather than some specific composite type.

Likewise for FieldStore.

Like the previous patch, back-patch to all supported branches.

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

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 3b214e57028574452d24a25f3230d33ac710cd5e..033c4358eabae5216556f8325c0afb976b6bdf1c 100644 (file)
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1716,10 +1716,24 @@ find_expr_references_walker(Node *node,
    else if (IsA(node, FieldSelect))
    {
        FieldSelect *fselect = (FieldSelect *) node;
+       Oid         argtype = getBaseType(exprType((Node *) fselect->arg));
+       Oid         reltype = get_typ_typrelid(argtype);
 
-       /* result type might not appear anywhere else in expression */
-       add_object_address(OCLASS_TYPE, fselect->resulttype, 0,
-                          context->addrs);
+       /*
+        * We need a dependency on the specific column named in FieldSelect,
+        * assuming we can identify the pg_class OID for it.  (Probably we
+        * always can at the moment, but in future it might be possible for
+        * argtype to be RECORDOID.)  If we can make a column dependency then
+        * we shouldn't need a dependency on the column's type; but if we
+        * can't, make a dependency on the type, as it might not appear
+        * anywhere else in the expression.
+        */
+       if (OidIsValid(reltype))
+           add_object_address(OCLASS_CLASS, reltype, fselect->fieldnum,
+                              context->addrs);
+       else
+           add_object_address(OCLASS_TYPE, fselect->resulttype, 0,
+                              context->addrs);
        /* the collation might not be referenced anywhere else, either */
        if (OidIsValid(fselect->resultcollid) &&
            fselect->resultcollid != DEFAULT_COLLATION_OID)
@@ -1729,10 +1743,20 @@ find_expr_references_walker(Node *node,
    else if (IsA(node, FieldStore))
    {
        FieldStore *fstore = (FieldStore *) node;
+       Oid         reltype = get_typ_typrelid(fstore->resulttype);
 
-       /* result type might not appear anywhere else in expression */
-       add_object_address(OCLASS_TYPE, fstore->resulttype, 0,
-                          context->addrs);
+       /* similar considerations to FieldSelect, but multiple column(s) */
+       if (OidIsValid(reltype))
+       {
+           ListCell   *l;
+
+           foreach(l, fstore->fieldnums)
+               add_object_address(OCLASS_CLASS, reltype, lfirst_int(l),
+                                  context->addrs);
+       }
+       else
+           add_object_address(OCLASS_TYPE, fstore->resulttype, 0,
+                              context->addrs);
    }
    else if (IsA(node, RelabelType))
    {

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 838588757a1b5ea925f769f3a4ba1ce56275294e..e12a1ac5cbf9c9e1ecbee34cf87b1803d67d3640 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2706,6 +2706,23 @@ Typed table of type: test_type2
 Inherits: test_tbl2
 
 DROP TABLE test_tbl2_subclass;
+CREATE TYPE test_typex AS (a int, b text);
+CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0));
+ALTER TYPE test_typex DROP ATTRIBUTE a; -- fails
+ERROR:  cannot drop composite type test_typex column a because other objects depend on it
+DETAIL:  constraint test_tblx_y_check on table test_tblx depends on composite type test_typex column a
+HINT:  Use DROP ... CASCADE to drop the dependent objects too.
+ALTER TYPE test_typex DROP ATTRIBUTE a CASCADE;
+NOTICE:  drop cascades to constraint test_tblx_y_check on table test_tblx
+\d test_tblx
+               Table "public.test_tblx"
+ Column |    Type    | Collation | Nullable | Default 
+--------+------------+-----------+----------+---------
+ x      | integer    |           |          | 
+ y      | test_typex |           |          | 
+
+DROP TABLE test_tblx;
+DROP TYPE test_typex;
 -- This test isn't that interesting on its own, but the purpose is to leave
 -- behind a table to test pg_upgrade with. The table has a composite type
 -- column in it, and the composite type has a dropped attribute.

diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 2ef9541a8c9aa30112ea5a59d4f5c282d931f593..f248f529256b8f7619d6271df5e63dca8c6d814c 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1714,6 +1714,14 @@ ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE;
 
 DROP TABLE test_tbl2_subclass;
 
+CREATE TYPE test_typex AS (a int, b text);
+CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0));
+ALTER TYPE test_typex DROP ATTRIBUTE a; -- fails
+ALTER TYPE test_typex DROP ATTRIBUTE a CASCADE;
+\d test_tblx
+DROP TABLE test_tblx;
+DROP TYPE test_typex;
+
 -- This test isn't that interesting on its own, but the purpose is to leave
 -- behind a table to test pg_upgrade with. The table has a composite type
 -- column in it, and the composite type has a dropped attribute.