[Tom Lane <tgl@sss.pgh.pa.us>]

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

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

Attachments:

fieldselect-dependencies-take-two.patch (text/x-diff, 2.7K) download
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))
  	{