Received: from malur.postgresql.org ([217.196.149.56])
	by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256)
	(Exim 4.89)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1iUHRU-0006aq-U7
	for pgsql-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 21:41:49 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.89)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1iUHRT-0002T6-0f
	for pgsql-hackers@arkaria.postgresql.org; Mon, 11 Nov 2019 21:41:47 +0000
Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29])
	by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256)
	(Exim 4.89)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1iUHRS-0002Sz-OO
	for pgsql-hackers@lists.postgresql.org; Mon, 11 Nov 2019 21:41:46 +0000
Received: from sss.pgh.pa.us ([66.207.139.130])
	by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256)
	(Exim 4.89)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1iUHRQ-0001Fv-6F
	for pgsql-hackers@lists.postgresql.org; Mon, 11 Nov 2019 21:41:46 +0000
Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1])
	by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id xABLffxr018428
	for <pgsql-hackers@lists.postgresql.org>; Mon, 11 Nov 2019 16:41:41 -0500
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Missing dependency tracking for TableFunc nodes
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <18269.1573508292.0@sss.pgh.pa.us>
Date: Mon, 11 Nov 2019 16:41:41 -0500
Message-ID: <18427.1573508501@sss.pgh.pa.us>
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <18269.1573508292.1@sss.pgh.pa.us>

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


------- =_aaaaaaaaaa0
Content-Type: text/plain; name="tablefunc-dependency-bug.sql";
	charset="us-ascii"
Content-ID: <18269.1573508292.2@sss.pgh.pa.us>
Content-Description: tablefunc-dependency-bug.sql

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;

------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-tablefunc-dependencies.patch";
	charset="us-ascii"
Content-ID: <18269.1573508292.3@sss.pgh.pa.us>
Content-Description: fix-tablefunc-dependencies.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/depend=
ency.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 =3D (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 =3D lfirst_oid(ct);
+
+			if (OidIsValid(collid) && collid !=3D DEFAULT_COLLATION_OID)
+				add_object_address(OCLASS_COLLATION, collid, 0,
+								   context->addrs);
+		}
+	}
 	else if (IsA(node, TableSampleClause))
 	{
 		TableSampleClause *tsc =3D (TableSampleClause *) node;

------- =_aaaaaaaaaa0--