Date: Mon, 10 Dec 2018 08:21:22 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
	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: <20181210132122.GI3415@tamriel.snowman.net>
References: <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>
 <16590.1544378743@sss.pgh.pa.us>
 <20181209185452.GF3415@tamriel.snowman.net>
 <19520.1544382449@sss.pgh.pa.us>
 <20181209223941.GA3710@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="jXPH6WnSavjjGXXC"
Content-Disposition: inline
In-Reply-To: <20181209223941.GA3710@paquier.xyz>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


--jXPH6WnSavjjGXXC
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Michael Paquier (michael@paquier.xyz) wrote:
> On Sun, Dec 09, 2018 at 02:07:29PM -0500, Tom Lane wrote:
> > Stephen Frost <sfrost@snowman.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 ti=
me
> >> to time, for example.  Perhaps we could have these functions run just
> >> once per relkind.
> >=20
> > 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 t=
he
> > 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.
>=20
> 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

--jXPH6WnSavjjGXXC
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJcDmhSAAoJEO1sijiDR2RVdoUP/3Tk6GfvOgzSZxzovzviU7tv
k9AzWQwKdMHw5hvl06fVvFUpSjD9vZwPnv6X0459ZYZNmVhMFS38P5mU7GY77VqY
IWE+dt1wCS9ZDLeQFTg4+TtUwhYAWzd3LwWePynuzAVGm891nGWgsBR8q7BQv8oG
7NlIoTFgJPHp0BvbUiaP5CEIPZZ1boI+jgCaOLYEwhX5VrOpmEZIXJBFaGx2oJXD
ge9y/8zBZgenV77fYUi55BNQbX0tvsU7Xn0L+Lj2KHOJEL8hrP9CM9xCIo+Nrhrq
H6qohuGSHO7u3CfqOEQm3MoOvGTqNYcqlp+Slm4mM79wiqk5J4Ks5WIRgy+wHON5
buD5Iby8V0dn2w7H9T1zDHeAu4enFeoug943xW4Rdokx14vci8jtNehljAe0xo4d
ZiFjoNyOcEE9De31N+OcaOYH5aF1nQEWZVYKWcrouUEJDwE6KH4/rZxmAtx0ciJK
Csji6Ymrx3vhb74MMT3u7XQziwqXQrtR2Yhr2p/ku67XCIqdJJofDslhPOldq7Cw
ne8TSlim1AkDXQAQddK7OhEG2CYMLPJ4A3ofixFgHvi51tuJaBbZHgaImpZWHPk4
m6FsipbNuj2/SItR/01h8pQiG6q824enoNOpfQWAyjG4Xr/rpJI7A0V91+qBHDlm
eb2sg8Yiusg3Jm1hnfpS
=p1Iz
-----END PGP SIGNATURE-----

--jXPH6WnSavjjGXXC--