Date: Thu, 28 Feb 2019 10:45:49 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Stephen Frost <sfrost@snowman.net>,
	Postgres hackers <pgsql-hackers@postgresql.org>,
	Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Subject: Re: pg_partition_tree crashes for a non-defined relation
Message-ID: <20190228014549.GD1617@paquier.xyz>
References: <19520.1544382449@sss.pgh.pa.us>
 <20190227184808.GA17357@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="wchHw8dVAp53YPj8"
Content-Disposition: inline
In-Reply-To: <20190227184808.GA17357@alvherre.pgsql>
Precedence: bulk


--wchHw8dVAp53YPj8
Content-Type: multipart/mixed; boundary="BRE3mIcgqKzpedwo"
Content-Disposition: inline


--BRE3mIcgqKzpedwo
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Feb 27, 2019 at 03:48:08PM -0300, Alvaro Herrera wrote:
> I just happened to come across the result of this rationale in
> pg_partition_tree() (an SRF) while developing a new related function,
> pg_partition_ancestors(), and find the resulting behavior rather absurd
> -- it returns one row with all NULL columns, rather than no rows.  I
> think the sensible behavior would be to do SRF_RETURN_DONE() before
> stashing any rows to the output, so that we get an empty result set
> instead.

Hmm.  Going through the thread again NULL was decided to make the
whole experience consistent, now by returning nothing we would get
a behavior as consistent as when NULL is used in input, so point taken
to tune the behavior for unsupported relkinds and undefined objects.

Does the attached look fine to you?
--
Michael

--BRE3mIcgqKzpedwo
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="partition-tree-srf.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/adt/partitionfuncs.c b/src/backend/utils/adt=
/partitionfuncs.c
index ffd66b6439..36d9f69cbc 100644
--- a/src/backend/utils/adt/partitionfuncs.c
+++ b/src/backend/utils/adt/partitionfuncs.c
@@ -69,9 +69,6 @@ pg_partition_tree(PG_FUNCTION_ARGS)
 	FuncCallContext *funcctx;
 	ListCell  **next;
=20
-	if (!check_rel_can_be_partition(rootrelid))
-		PG_RETURN_NULL();
-
 	/* stuff done only on the first call of the function */
 	if (SRF_IS_FIRSTCALL())
 	{
@@ -82,6 +79,9 @@ pg_partition_tree(PG_FUNCTION_ARGS)
 		/* create a function context for cross-call persistence */
 		funcctx =3D SRF_FIRSTCALL_INIT();
=20
+		if (!check_rel_can_be_partition(rootrelid))
+			SRF_RETURN_DONE(funcctx);
+
 		/* switch to memory context appropriate for multiple function calls */
 		oldcxt =3D MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
=20
diff --git a/src/test/regress/expected/partition_info.out b/src/test/regres=
s/expected/partition_info.out
index a884df976f..73269ffd09 100644
--- a/src/test/regress/expected/partition_info.out
+++ b/src/test/regress/expected/partition_info.out
@@ -9,8 +9,7 @@ SELECT * FROM pg_partition_tree(NULL);
 SELECT * FROM pg_partition_tree(0);
  relid | parentrelid | isleaf | level=20
 -------+-------------+--------+-------
-       |             |        |     =20
-(1 row)
+(0 rows)
=20
 SELECT pg_partition_root(NULL);
  pg_partition_root=20
@@ -163,14 +162,12 @@ CREATE MATERIALIZED VIEW ptif_test_matview AS SELECT =
1;
 SELECT * FROM pg_partition_tree('ptif_test_view');
  relid | parentrelid | isleaf | level=20
 -------+-------------+--------+-------
-       |             |        |     =20
-(1 row)
+(0 rows)
=20
 SELECT * FROM pg_partition_tree('ptif_test_matview');
  relid | parentrelid | isleaf | level=20
 -------+-------------+--------+-------
-       |             |        |     =20
-(1 row)
+(0 rows)
=20
 SELECT pg_partition_root('ptif_test_view');
  pg_partition_root=20

--BRE3mIcgqKzpedwo--

--wchHw8dVAp53YPj8
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAlx3PU0ACgkQnvQgOdby
QH0t5w//fPNx1CWwJWUHeC7uavVcXJIrGN/c1mk11PBqK5vPnWvsQ/EqMB4r9k7e
20qnhkyCVxhCGMtjMZFDz3A4lBjA83iowws/XOsGzOZ6LN8Zq8yUPjkO/rEo/7rG
+Nz+wbAVmHrK4SBCpwXaJ56V9Gv4g4t1I+wmcOKBjAPW5pTI0JArpQJ5qZP9Xkua
fwwhO1ZySuPP0YRNnu8XgWJypGen2cYjynqPUrnVzr2ot2jJgA/JCOa72RwUoQNw
BzSRxhU34zXDH0nbjkC3PJJRa0mEL+JxuDGA0l2tYg8gsCkp+9mhhTKvSq9Neczu
aYuiQ6xxFXYy4MR+x3wVU4QXWQXa/8mn26tNtokaFO1ZafruQzIXe//s1X0q4Meo
crT0mZWL2StR///GrRmd61HdFQRvjnWoTiHMtz3Jz3v7t829Ht9YGEql8bF9MySy
0snkAernnN8wZLbeFRxhXe4Oble5SZ5dtKLtzj0alqGA4HB7Zn2/1bzvk4q1OiMw
fAtX2HKahmeprIg36bHgE4+Y+QpPgWD9g8f17tkZP37t89zu8yH56BNdD0ONWl4q
iAfsboCbenWlhBkk60dLswWo+FDhxMdm08tcz5uxCS5H1UktWBvhbRPqiq2g0nd6
9II+0Tvb2LMyF43N3mWaej+BEqccFR0lArAT254F2Kydj4FaDQg=
=SnAA
-----END PGP SIGNATURE-----

--wchHw8dVAp53YPj8--