Date: Wed, 10 Oct 2018 13:01:50 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Jesper Pedersen <jesper.pedersen@redhat.com>,
	Thomas Munro <thomas.munro@enterprisedb.com>,
	Robert Haas <robertmhaas@gmail.com>,
	Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>,
	Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: partition tree inspection functions
Message-ID: <20181010040150.GD2204@paquier.xyz>
References: <20181005065219.GD14664@paquier.xyz>
 <5aa10b81-1d56-2983-9c2b-bc2164f6a803@redhat.com>
 <20181006062631.GB4835@paquier.xyz>
 <1696808d-fa0d-2e8b-bc20-722616758353@lab.ntt.co.jp>
 <20181009091000.GA1582@paquier.xyz>
 <fbef2be7-61fd-7c9f-f780-1d6b20ff9eb2@lab.ntt.co.jp>
 <20181009100544.GB1582@paquier.xyz>
 <f4639525-fe10-cf47-d1a2-d4df7af74d90@lab.ntt.co.jp>
 <20181009111720.GC1582@paquier.xyz>
 <cffbde33-f355-93dc-3e60-56324ece945d@lab.ntt.co.jp>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="Qz2CZ664xQdCRdPu"
Content-Disposition: inline
In-Reply-To: <cffbde33-f355-93dc-3e60-56324ece945d@lab.ntt.co.jp>
Precedence: bulk


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

On Wed, Oct 10, 2018 at 11:54:48AM +0900, Amit Langote wrote:
> I was partly wrong in saying that we wouldn't need any changes to support
> partitioned indexes here.  Actually, the core function
> find_inheritance_children wouldn't scan pg_inherits for us if we pass an
> (partitioned) index to it, even if the index may have children.
>
> It appears that we don't set relhassubclass for partitioned indexes (that
> is, on parent indexes), so the above the test is useless for indexes.

Aha, so that was it.

> Maybe, we need to start another thread to discuss whether we should be
> manipulating relhassubclass for index partitioning (I'd brought this up in
> the context of relispartition, btw [1]).  I'm not immediately sure if
> setting relhassubclass correctly for indexes will have applications beyond
> this one, but it might be something we should let others know so that we
> can hear more opinions.

This reminds of https://postgr.es/m/20180226053613.GD6927@paquier.xyz,
which has resulted in relhaspkey being removed from pg_class with
f66e8bf8.  I would much prefer if we actually removed it...  Now
relhassubclass is more tricky than relhaspkey as it gets used as a
fast-exit path for a couple of things:
1) prepunion.c when planning for child relations.
2) plancat.c
2) analyze.c
3) rewriteHandler.c

> For time being, I've modified that code as follows:
>=20
> @@ -68,9 +70,11 @@ find_inheritance_children(Oid parentrelId, LOCKMODE
> lockmode)
>=20
>      /*
>       * Can skip the scan if pg_class shows the relation has never had a
> -     * subclass.
> +     * subclass.  Since partitioned indexes never have their
> +     * relhassubclass set, we cannot skip the scan based on that.
>       */
> -    if (!has_subclass(parentrelId))
> +    if (get_rel_relkind(parentrelId) !=3D RELKIND_PARTITIONED_INDEX &&
> +        !has_subclass(parentrelId))
>          return NIL;

I don't like living with this hack.  What if we simply nuked this
fast-path exit all together?  The only code path where this could
represent a performance issue is RelationBuildPartitionKey(), but we
expect a partitioned table to have children anyway, and there will be
few cases where partitioned tables have no partitions.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAlu9ea4ACgkQnvQgOdby
QH1O+A/+LvAgmQiJXSk8CbgoWoMbnWVrz/pq3ma26FaQ0tvB0g1KE9/d8fmEj+wp
qkOv+GQvpVVd1gJ/2B3XffJFMS0bVCqA2Hp+Eu+1k4rp2uujYgMTtY9U2sT1VuNT
YNsNEKtPXHVte/ShZsIcJ0fCC04kEwy4f5aw181PMPK72apIsmMoOVEJFgn4pna2
8Je/3G38gjyCXU4iU3iHTnhsqRjalAKSVk2USksh1PgePw35J0lkfKigH4aPZ0iP
Wei3TOiBy3NGcWdjo6YSyxDyVQ2Zk2lEggUvchOM6jgv8/4kLcZL0zNSU0M80xRY
DkL9IgGLA3PxT2B/9wkPl9WNbLr/QQhamX1nVodq+i2qJkHezIeXR3PSWQhw9TqO
x9PvB+2y8TdXDTKdD3j89pegSX764Jv1JikkMEB21rI7b1crpxikVL6s8BAsG4FT
jA5ywdt3ARL0B7ko1Sl6g+9UQR0WYGuY6HoIkSxfI0KgagIETOAD3xyoEshIsP16
fr060IGtsTEQ85tbUdXZkVVYRgQ4iXsgfZDQ6RW0wmiDCQoVMWiJFgXCPeu6gdCe
x7g7cns7zO2nSCey7UAYO60hxS00C1fn9OOv4uRuRtWG62H1oT0pA6g34SFghxNL
9xr1AqNBy6aKrgdln3SHLGIRPwnOczExy6lezLR6kuI74H2ewMc=
=Ys4K
-----END PGP SIGNATURE-----

--Qz2CZ664xQdCRdPu--