Re: pg_partition_tree crashes for a non-defined relation

Greetings,

* Michael Paquier (michael(at)paquier(dot)xyz) wrote:
> On Sun, Dec 09, 2018 at 02:07:29PM -0500, Tom Lane wrote:
> > Stephen Frost <sfrost(at)snowman(dot)net> writes:
> >> I don't entirely buy off on the argument that it's code that's 'highly
> >> unlikely to break once written' though- we do add new relkinds from time
> >> to time, for example. Perhaps we could have these functions run just
> >> once per relkind.
> >
> > Well, the relevant code is likely to be "if relkind is not x, y, or z,
> > then PG_RETURN_NULL". If we add a new relkind and forget to consider the
> > function, the outcome is a NULL result that perhaps should not have been
> > NULL ... but a test like this won't help us notice that.
>
> Yes, in order to prevent problems with newly-introduced relkinds, I
> think that the checks within functions should be careful to check only
> for relkinds that they support, and not list those they do not support.

Yes, but it's certainly possible for someone to add another relkind to
that list without looking through the rest of the function carefully
enough. Perhaps not the best example. I still don't like the
assumption that the code won't be broken once it's written correctly the
first time though, and I continue to feel that it would be good to have
regression tests which run these functions with interesting arguments.

I also agree that we don't want to make the regression tests take a lot
of additional time.

Thanks!

Stephen