From: Tom Lane <tgl@sss.pgh.pa.us>
To: Stephen Frost <sfrost@snowman.net>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@postgresql.org>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Subject: Re: pg_partition_tree crashes for a non-defined relation
Date: Sun, 09 Dec 2018 13:05:43 -0500
Message-ID: <16590.1544378743@sss.pgh.pa.us> (raw)
In-Reply-To: <20181209174815.GD3415@tamriel.snowman.net>
References: <20181207010406.GO2407@paquier.xyz>
<20181207230540.GA12740@paquier.xyz>
<6105.1544243612@sss.pgh.pa.us>
<20181208065436.GC1989@paquier.xyz>
<20181208134608.GY3415@tamriel.snowman.net>
<20181208231507.GA1833@paquier.xyz>
<20181209174815.GD3415@tamriel.snowman.net>
Stephen Frost <sfrost@snowman.net> writes:
> * Michael Paquier (michael@paquier.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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, sfrost@snowman.net, michael@paquier.xyz, Langote_Amit_f8@lab.ntt.co.jp
Subject: Re: pg_partition_tree crashes for a non-defined relation
In-Reply-To: <16590.1544378743@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox