From f3ea3e3e820b6b7512e48660bf984603418d53ff Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 23 Oct 2017 13:57:45 -0400 Subject: [PATCH] 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 --- src/backend/catalog/dependency.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 2668650f27..3b214e5702 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -1713,6 +1713,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; @@ -1733,6 +1754,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)) {