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