Hi, On 2019/03/01 9:22, Michael Paquier wrote: > On Thu, Feb 28, 2019 at 04:32:03PM -0300, Alvaro Herrera wrote: >> Yeah, looks good, please push. > > Done for this part. > >> 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. > > I think that one option is to make the function return only the table > itself if it is not a partitioned table, which would be more > consistent with what pg_partition_root() does. > > What I am writing next sounds perhaps a bit fancy, but in my opinion a > normal table is itself a partition tree, made of one single member: > itself.
That's what we discussed, but it seems that we ended up allowing regular standalone tables (possibly in inheritance trees) only because it *appeared* to work. Alvaro already pointed out what appears to be a bug in how we compute the value of level. Instead of trying to fix it, I agree we should just disallow tables that are not a partitioned table/index or a partition (relispartition). Maybe there won't be any use cases, so we should change that while we still can. So, maybe change the check in check_rel_can_be_partition() as follows: relkind = get_rel_relkind(relid); relispartition = get_rel_relispartition(relid); /* Only allow relation types that can appear in partition trees. */ if (!relispartition && relkind != RELKIND_PARTITIONED_TABLE && relkind != RELKIND_PARTITIONED_INDEX) return false; Thanks, Amit