Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 29 Jun 2023 19:39:39 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17997: Assert failed in validatePartitionedIndex() when
 attaching partition index to child of valid index
Message-ID: <ZJ1fa/kwMsAvqZit@paquier.xyz>
References: <17997-a044c27aef95daf8@postgresql.org>
 <509915.1687704058@sss.pgh.pa.us>
 <ZJk04dp0mz7s58Lx@paquier.xyz>
 <ZJlG0a/F03iPI/wj@paquier.xyz>
 <14987634-43c0-0cb3-e075-94d423607e08@gmail.com>
 <3eaa2c33-2ed1-2996-c6b6-0e7552a64cae@gmail.com>
 <ZJva9997u5H0RQe1@paquier.xyz>
 <611182a0-172e-cb80-965d-d6c69a1bde62@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="+11wT7ueAiVyOz8e"
Content-Disposition: inline
In-Reply-To: <611182a0-172e-cb80-965d-d6c69a1bde62@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZJ1fa/kwMsAvqZit%40paquier.xyz>
Precedence: bulk


--+11wT7ueAiVyOz8e
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Jun 29, 2023 at 12:00:00PM +0300, Alexander Lakhin wrote:
> That is, the partition tree is containing no invalid indexes now, but the
> upper-level indexes in the tree are still invalid.
> Moreover, I don't know how to make them valid:
> reindex index parted_isvalid_idx; / reindex index parted_isvalid_tab_1_ex=
pr_idx;
> doesn't affect their flags indisvalid.
> (Though REINDEX for a top-level index can make leaf indexes valid.)
> Reattaching parted_isvalid_tab_11 after "update parted_isvalid_tab_11 set=
 b=3D1"
> doesn't help either.

DETACH PARTITION is not possible for indexes, but you should be able
to get back to a cleaner tree by detaching a partition, dropping an
index, and re-attaching back the partitions, so as
validatePartitionedIndex() is able to trigger a refresh of=20
indisvalid depending on the state of the tree.  Yeah, that's annoying,
and things are what they are for a few years now.  It does not seem
like we've looked at how all that should behave in combination with
concurrent index builds, for example, that can lead to a transient
state of a tree.

> So the above test case makes me wonder, is it realistic in principle to
> maintain this definition of indisvalid up-to-date for a partition
> tree?
>
> So I can't resist asking a question: what would break if indisvalid flags
> were always true (or always false) for the upper-level partition indexes?
> I've made validatePartitionedIndex() a no-op just to check that and the
> only differences I observe doing `make check` are indisvalid flags printe=
d;
> there are no behavioral changes.
> Though `make check-world` fails on src/bin/pg_upgrade (only); I need more
> time to investigate this =E2=80=94 maybe more sophisticated query to sele=
ct
> appropriate indexes for processing could help there.

It offers a hint to the planner about which indexes should be used,
but I would spend a bit more time on plancat.c to see the implications
of that for partitioned indexes.  validatePartitionedIndex() has a
clean implied definition, at least.

Anyway, DefineIndex() and its handling of indisvalid is clearly wrong
when it comes to multiple partition layers as it forgets that we may
have to update the parents recursively or we could once again trigger
the assertion from validatePartitionedIndex(), which would not be a
good idea..  So my previous patch fixes that, at least.
--
Michael

--+11wT7ueAiVyOz8e
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmSdX2sACgkQnvQgOdby
QH2V4BAAlQEGluuyvrjGnImHtyQ2F87cJDAKxwlRrnjQM7OnU8tXLD+FlL0xa+Xi
iW3lxdGML0JsOTjdmPSO+W7N823/9j1R2NFUdUj79RoPha+CVF9nqmPDycI89JHN
N5GpYMS9yxFi201KHiSA0oRmeWe9Qfm7/abKcLlmwmfIGpiHO6McVzUSGh6cD6JC
xXdnJTCbMfYt6a9MKoV4UazWOoM1NU2ODjJNTXKOMFy7DFnEwSv6C0gCSfp1/aV6
YSN+fPSOFT6S9EXz/5uRd0dN+u94LCh5tsxsguoBNPnFQKJbVGsc4ScroF0aL3ns
jc1QokQaUWgq6nRNf3Y/Bz9heLB/9zloPYcFMTMdd38zVIMb9etzVDP5N1U75rRH
WRWwLiSiKNZ/sMxrrpvLvlAk3FuxDizSZ6CEpkgdiLOPOTvbnTFrCCxBwH/OSI1+
AFfAyN7yE7NJE5WciOxOsQ5FXQxK5YndGDftLTtM2Qd/L1YLwRxwcqMzVNSwgMXy
QY5Jp/uc/MLg3DwsnjSDwgfNXYPkFuaMV7ZWit511jLb7w6g+1N1PJv/taLxPy6O
bops6v54RrNDpz9drenKm9MBCG+cEbKtZc0Lts5NIEWPxq0MYv0uM7ZA59TeBtvA
RsPzJbeCVM6oubytyikwT8bzO3G+wYXKCC/nzos7lbx5MeOXpZc=
=snrr
-----END PGP SIGNATURE-----

--+11wT7ueAiVyOz8e--