Re: pg_partition_tree crashes for a non-defined relation

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Michael Paquier (michael(at)paquier(dot)xyz) wrote:
>> On Sat, Dec 08, 2018 at 08:46:08AM -0500, Stephen Frost wrote:
>>> I wonder if we maybe should have a regression test for every such
>>> function which just queries the catalog in a way to force the function
>>> to be called for every relation defined in the regression tests, to
>>> ensure that it doesn't segfault or throw an error..

> No, I mean something like:
> with x as (select pg_partition_tree(relname) from pg_class)
> select 1 from x limit 1;
> or whatever it takes to make sure that the function is run against every
> entry in pg_class (or whatever is appropriate) while not returning the
> results (since we don't actually care about the output, we just want to
> make sure it doesn't ERROR or crash).

I'm going to object to that on cost grounds. It is not reasonable to
run moderately-expensive functions like this on more than a thousand
pg_class entries in order to test what are just a few distinct cases,
especially in code that's highly unlikely to break once written.
Or at least, it's not reasonable to do that every time anybody runs
the regression tests for the rest of our lives.

Moreover, this would only help check a specific new function if the
author or reviewer remembered to add a test case that does it.
Since the whole problem here is "I forgot to consider this", I don't
have much faith in that happening.

Maybe we should have some sort of checklist of things to think about
when adding new SQL-visible functions, rather than trying to
memorialize every such consideration as a regression test no matter
how expensive. Admittedly, "I forgot to go over the checklist" is
still a problem; but at least it's not penalizing every developer and
every buildfarm run till kingdom come.

regards, tom lane