Fix some oversights in expression dependency recording.

find_expr_references() neglected to record a dependency on the result type
of a FieldSelect node, allowing a DROP TYPE to break a view or rule that
contains such an expression.  I think we'd omitted this case intentionally,
reasoning that there would always be a related dependency ensuring that the
DROP would cascade to the view.  But at least with nested field selection
expressions, that's not true, as shown in bug #14867 from Mansur Galiev.
Add the dependency, and for good measure a dependency on the node's exposed
collation.

Likewise add a dependency on the result type of a FieldStore.  I think here
the reasoning was that it'd only appear within an assignment to a field,
and the dependency on the field's column would be enough ... but having
seen this example, I think that's wrong for nested-composites cases.

Looking at nearby code, I notice we're not recording a dependency on the
exposed collation of CoerceViaIO, which seems inconsistent with our choices
for related node types.  Maybe that's OK but I'm feeling suspicious of this
code today, so let's add that; it certainly can't hurt.

This patch does not do anything to protect already-existing views, only
views created after it's installed.  But seeing that the issue has been
there a very long time and nobody noticed till now, that's probably good
enough.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/20171023150118.1477.19174@wrigleys.postgresql.org

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index f71d80fc1a1fc4a41a4023c1ebafc51908e04f4f..eec66d20116267684a5c944253c11c84b3731561 100644 (file)
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1710,6 +1710,27 @@ find_expr_references_walker(Node *node,
                /* Extra work needed here if we ever need this case */
                elog(ERROR, "already-planned subqueries not supported");
        }
+       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)
+                       add_object_address(OCLASS_COLLATION, fselect->resultcollid, 0,
+                                                          context->addrs);
+       }
+       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))
        {
                RelabelType *relab = (RelabelType *) node;
@@ -1730,6 +1751,11 @@ find_expr_references_walker(Node *node,
                /* since there is no exposed function, need to depend on type */
                add_object_address(OCLASS_TYPE, iocoerce->resulttype, 0,
                                                   context->addrs);
+               /* the collation might not be referenced anywhere else, either */
+               if (OidIsValid(iocoerce->resultcollid) &&
+                       iocoerce->resultcollid != DEFAULT_COLLATION_OID)
+                       add_object_address(OCLASS_COLLATION, iocoerce->resultcollid, 0,
+                                                          context->addrs);
        }
        else if (IsA(node, ArrayCoerceExpr))
        {