I happened to notice that find_expr_references_walker has not
been taught anything about TableFunc nodes, which means it will
miss the type and collation OIDs embedded in such a node.

This can be demonstrated to be a problem by the attached script,
which will end up with a "cache lookup failed for type NNNNN"
error because we allow dropping a type the XMLTABLE construct
references.

This isn't hard to fix, as per the attached patch, but it makes
me nervous.  I wonder what other dependencies we might be missing.

Would it be a good idea to move find_expr_references_walker to
nodeFuncs.c, in hopes of making it more visible to people adding
new node types?  We could decouple it from the specific use-case
of recordDependencyOnExpr by having it call a callback function
for each identified OID; although maybe there's no point in that,
since I'm not sure there are any other use-cases.

Another thought is that maybe the code could be automatically
generated, as Andres has been threatening to do with respect
to the other stuff in backend/nodes/.

In practice, this bug is probably not a huge problem, because a
view that involves a column of type X will likely have some other
dependencies on X.  I had to tweak the example view a bit to get
it to not have any other dependencies on "seg".  So I'm not feeling
that this is a stop-ship problem for today's releases --- I'll plan
on installing the fix after the releases are tagged.

                        regards, tom lane

CREATE EXTENSION seg;

DROP TABLE IF EXISTS xmldata CASCADE;

CREATE TABLE xmldata AS SELECT
xml $$
<ROWS>
  <ROW id="1">
    <LOC>0 .. 1</LOC>
    <COUNTRY_NAME>Australia</COUNTRY_NAME>
  </ROW>
  <ROW id="5">
    <LOC>0 .. 4</LOC>
    <COUNTRY_NAME>Japan</COUNTRY_NAME>
  </ROW>
</ROWS>
$$ AS data;

CREATE VIEW xmlview AS
SELECT xmltable.id, "LOC"::text
  FROM xmldata,
       XMLTABLE('//ROWS/ROW'
                PASSING data
                COLUMNS id int PATH '@id',
                        "COUNTRY_NAME" text,
                        "LOC" seg);

SELECT * FROM xmlview;

DROP EXTENSION seg;

SELECT * FROM xmlview;
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 0358278..d07bb44 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -2256,6 +2256,28 @@ find_expr_references_walker(Node *node,
 								   context->addrs);
 		}
 	}
+	else if (IsA(node, TableFunc))
+	{
+		TableFunc  *tf = (TableFunc *) node;
+		ListCell   *ct;
+
+		/*
+		 * Add refs for the datatypes and collations used in the TableFunc.
+		 */
+		foreach(ct, tf->coltypes)
+		{
+			add_object_address(OCLASS_TYPE, lfirst_oid(ct), 0,
+							   context->addrs);
+		}
+		foreach(ct, tf->colcollations)
+		{
+			Oid			collid = lfirst_oid(ct);
+
+			if (OidIsValid(collid) && collid != DEFAULT_COLLATION_OID)
+				add_object_address(OCLASS_COLLATION, collid, 0,
+								   context->addrs);
+		}
+	}
 	else if (IsA(node, TableSampleClause))
 	{
 		TableSampleClause *tsc = (TableSampleClause *) node;

Reply via email to