[HACKERS] FieldSelect/FieldStore dependencies, take 2 - Mailing list pgsql-hackers

I had a nagging feeling that commit f3ea3e3e8 was not quite covering
all the bases with respect to what dependencies to record for
FieldSelect/FieldStore nodes: it looked at the result type, but what
about the input type?  Just now, while fooling around with domains
over composite, I stumbled across a case that shows what's missing:

regression=# create type complex as (r float8, i float8);
CREATE TYPE
regression=# create domain dcomplex as complex check((value).r > (value).i);
CREATE DOMAIN
regression=# select pg_get_constraintdef((select max(oid) from pg_constraint));
      pg_get_constraintdef
---------------------------------
 CHECK (((VALUE).r > (VALUE).i))
(1 row)

regression=# alter type complex drop attribute r;
ALTER TYPE
regression=# select pg_get_constraintdef((select max(oid) from pg_constraint));
                     pg_get_constraintdef
--------------------------------------------------------------
 CHECK (((VALUE)."........pg.dropped.1........" > (VALUE).i))
(1 row)

Nothing seems to crash at this point, probably because we insert
nulls into dropped columns, so the CHECK just sees a NULL value
for "r" whenever it runs.  But obviously, the next dump/reload
or pg_upgrade is not going to end well.

So what this shows is that we need a dependency on the particular
column named by the FieldSelect or FieldStore.  Under normal
circumstances, that obviates the need for a dependency on the
FieldSelect's result type, which would match the column type.
I think concretely what we need is the attached.

(BTW, the getBaseType() is only necessary in HEAD, since before
domains-over-composites the argument of a FieldSelect couldn't
be a domain type.)

            regards, tom lane

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 3b214e5..033c435 100644
*** a/src/backend/catalog/dependency.c
--- b/src/backend/catalog/dependency.c
*************** find_expr_references_walker(Node *node,
*** 1716,1725 ****
      else if (IsA(node, FieldSelect))
      {
          FieldSelect *fselect = (FieldSelect *) node;

!         /* result type might not appear anywhere else in expression */
!         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)
--- 1716,1739 ----
      else if (IsA(node, FieldSelect))
      {
          FieldSelect *fselect = (FieldSelect *) node;
+         Oid            argtype = getBaseType(exprType((Node *) fselect->arg));
+         Oid            reltype = get_typ_typrelid(argtype);

!         /*
!          * 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)
*************** find_expr_references_walker(Node *node,
*** 1729,1738 ****
      else if (IsA(node, FieldStore))
      {
          FieldStore *fstore = (FieldStore *) node;

!         /* result type might not appear anywhere else in expression */
!         add_object_address(OCLASS_TYPE, fstore->resulttype, 0,
!                            context->addrs);
      }
      else if (IsA(node, RelabelType))
      {
--- 1743,1762 ----
      else if (IsA(node, FieldStore))
      {
          FieldStore *fstore = (FieldStore *) node;
+         Oid            reltype = get_typ_typrelid(fstore->resulttype);

!         /* 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))
      {

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers