Date: Wed, 9 Dec 2020 15:37:50 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>,
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: get_constraint_index() and conindid
Message-ID: <X9BwvuLuWJrIreQE@paquier.xyz>
References: <4688d55c-9a2e-9a5a-d166-5f24fe0bf8db@enterprisedb.com>
 <CAEze2WjBDFL3tSUTkaijZOa-pg3k_VcmyQzMXtW-qWQNfj-SKw@mail.gmail.com>
 <3260997.1607452119@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="VoBL+xf//iU561Q4"
Content-Disposition: inline
In-Reply-To: <3260997.1607452119@sss.pgh.pa.us>
Precedence: bulk


--VoBL+xf//iU561Q4
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Dec 08, 2020 at 01:28:39PM -0500, Tom Lane wrote:
> Matthias van de Meent <boekewurm+postgres@gmail.com> writes:
>> On Mon, 7 Dec 2020 at 11:09, Peter Eisentraut
>> <peter.eisentraut@enterprisedb.com> wrote:
>>> get_constraint_index() does its work by going through pg_depend.  It was
>>> added before pg_constraint.conindid was added, and some callers are
>>> still not changed.  Are there reasons for that?  Probably not.  The
>>> attached patch changes get_constraint_index() to an lsyscache-style
>>> lookup instead.
>=20
>> This looks quite reasonable, and it passes "make installcheck-world".
>=20
> +1, LGTM.

Nice cleanup!

>> Only thing I could think of is that it maybe could use a (small)
>> comment in the message on that/why get_constraint_index is moved to
>> utils/lsyscache from catalog/dependency, as that took me some time to
>> understand.
>=20
> commit message could reasonably say that maybe, but I don't think we
> need to memorialize it in a comment.  lsyscache.c *is* where one
> would expect to find a simple catalog-field-fetch function like this.
> The previous implementation was not that, so it didn't belong there.

Agreed.
--
Michael

--VoBL+xf//iU561Q4
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl/QcL4ACgkQnvQgOdby
QH2+Uw/+JZVJTeGTZUEit0mlWP7LpbZ39V18nonGI9v/hWnuXHMPnhUlCGG+8diX
ZfwObRmZmTtAjYLuJcyBcBANKY6SjdH6deZa0hPmEK0o4JJt+sw8VbRzj7XqwjmR
DPrMDsJJVb69mSPccp7Tb+VhhfqIDdvLERl49kdwLJgzQv/C3gvs3FPL3QFA8heH
p3iQnVf/RDQ1nB2vrMfWP87GQRTf7npmSUmWdpPtyBKHWdsXx4CWdbxmXXSYlQ0+
32vcn2IGlrszPafNTV2quqJBxgFTZFQ8Hx+JMMBwy2Wm5Te3ntY0ID+AHn4O3kad
FXQCoCi99S3ONjwdsqDFRszNQ2/adA4xbcBc+KpTaubntyVhO+AIXNRCBxmAPYwo
mhVbK1cfFUyNvDgWxjJS46TeFLlf9WI+Qn9lbszgnyA5p0eFiNCpc+4d1/Q0Jx2d
MqM60Ip0xcOoRaREKdJEAhtkByvvZe/8YjDRRtvIHZ4qjEkXZ/MgaKD4bggLbDso
lngkcGDPRorkVuNdBoHravtbWi+lFeHJ2AOQJ5OZqatSB/rLsqa8JHnrK/rtV8gF
ccWpkp+BvOpqd/SLFvHiJ0fIB6r6yAsz/Pb1MllzhmulyWzeo9JlpJxlplrB5DgF
aMgTfNlwJlO1/72eAexfxkqDISeGsAhvg7j/mf4cXRPMofjdJh0=
=ljeI
-----END PGP SIGNATURE-----

--VoBL+xf//iU561Q4--