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