Date: Thu, 28 Feb 2019 16:32:03 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Michael Paquier <michael@paquier.xyz>
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: <20190228193203.GA26151@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190228014549.GD1617@paquier.xyz>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On 2019-Feb-28, Michael Paquier wrote:

> 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.

Right, thanks.

> Does the attached look fine to you?

Yeah, looks good, please push.

What about legacy inheritance?  I see that pg_partition_tree handles
that case perfectly well -- it seems to return the complete hierarchy
rooted at the given relation.  However, it seems odd that it works at
all, don't you think?  Consider this:

create table t1 (a int);
create table t11 () inherits (t1);
create table t2 (b int);
create table t111() inherits (t1, t2);

alvherre=# select * from pg_partition_tree('t1');
 relid | parentrelid | isleaf | level 
-------+-------------+--------+-------
 t1    | t           | t      |     0
 t11   | t1          | t      |     1
 t111  | t1          | t      |     1
(3 filas)

OK so far... but look at t2's tree:

alvherre=# select * from pg_partition_tree('t2');
 relid | parentrelid | isleaf | level 
-------+-------------+--------+-------
 t2    | t           | t      |     0
 t111  | t1          | t      |     2

I think this one is just weird -- t1 is not listed as "relid" anywhere,
and why does t111 has level=2?

I would opt for returning the empty set for legacy inheritance too.

More generally, I think we should return empty for anything that's
either not RELKIND_PARTITIONED_TABLE or has relispartition set.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services