Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 28 Jun 2023 16:02:15 +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: <ZJva9997u5H0RQe1@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>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="F/pwlVaeN43cP1D5"
Content-Disposition: inline
In-Reply-To: <3eaa2c33-2ed1-2996-c6b6-0e7552a64cae@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZJva9997u5H0RQe1%40paquier.xyz>
Precedence: bulk


--F/pwlVaeN43cP1D5
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Jun 28, 2023 at 09:00:00AM +0300, Alexander Lakhin wrote:
> There is also another scenario where the new behavior could be
> considered as more sensible:
> create table t(a int, b int) partition by range (a);
> create index on t((a / b));
> create table tp1(a int, b int);
> insert into tp1 values (1, 0);
> create index concurrently on tp1((a/b)); --=A0 division by zero occurs, b=
ut the index is created (as invalid)
> alter table t attach partition tp1 for values from (1) to (10);
>=20
> Without the fix you get partition tp1_1 attached and the following partit=
ion indexes:
> Partitioned index "public.t_expr_idx"
> btree, for table "public.t"
> Partitions: tp1_expr_idx
>=20
> Index "public.tp1_expr_idx"
> btree, for table "public.tp1", invalid
>=20
> But with the patch applied ATTACH PARTITION fails with ERROR: division by=
 zero.

This makes rather sense to me.  You cannot really attach the index
in this case.  And the partition trees are clean.

> Though we still can get a partition index chain with invalid indexes
> as follows:
> create table t(a int, b int) partition by range (a);
> create table tp1(a int, b int) partition by range (a);
> alter table t attach partition tp1 for values from (1) to (100);
> create table tp1_1(a int, b int);
> insert into tp1_1 values (1, 0);
> create index concurrently on tp1_1((a/b)); -- division by zero occurs, bu=
t the index is created (as invalid)
> alter table tp1 attach partition tp1_1 for values from (1) to (10);
> create index on t((a / b));

Exotic case, leading to this tree:
=3D# select indisvalid, relid, parentrelid, isleaf, level
   from pg_partition_tree('t_expr_idx'), pg_index where indexrelid =3D reli=
d;
 indisvalid |     relid      | parentrelid  | isleaf | level=20
------------+----------------+--------------+--------+-------
 t          | t_expr_idx     | null         | f      |     0
 f          | tp1_expr_idx   | t_expr_idx   | f      |     1
 f          | tp1_1_expr_idx | tp1_expr_idx | t      |     2
(3 rows)

This looks like a second bug to me in DefineIndex() where we shouldn't
even try to link to the invalid index tp1_1_expr_idx, and we should
try to create a new index instead.  Repeating the last CREATE INDEX
command leads to a failure, which is what I'd expect, because we get
the computation failure.

> It's also interesting that REINDEX for the index tree validates only
> a leaf index:
> reindex index t_expr_idx;
> ERROR:=A0 division by zero

REINDEX on a partitioned index only works on the partitions, it does
nothing for the partitioned indexes.

> So it's not clear (to me, at least), what exactly indisvalid means
> for indexes in a partition tree.

It seems to me that validatePartitionedIndex() is what defines the
use of the flag for partitioned indexes.  indisvalid =3D false for a
partitioned index means that at least one of its partitions *may* not
have a valid index mapping to it, and that it should be switched to
true once we are sure that all its partitions have a valid index.
indisvalid =3D true on a partitioned table has much a stronger meaning,
where all the partitions *have* to have valid indexes.  Your first
case would follow that, but not the second.

For now, I have applied a patch down to v11 to fix the first issue for
the partition attaching.  Digging into the second one..
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmSb2vcACgkQnvQgOdby
QH1ZjA/+JwhM0Ju3mq5XSjU6hkdPhys0tuzr0qPYTXB8AyebHL1ehg4w9AQBhmUf
dCuV1WrQu64jVBkDeh3V/6Ai7DBr13TLxyQIQrvJmywerDIsjkvaNzsdbEWDBpG6
mn5jcdOo40bvS5KpAXBMLSvkZGjhdKqVSQzgy5FyiA+83F62Lxx56V8GG/zgY55m
RKDWvFIxWpkTZ3f/Kgc5gLPGpatnWiLXGTz9L+UcRSlKPewJTL4VOts2p1NbVy+P
tJGXcAWrhkCnaRlpq5lwiPcegBwahYEVyjifvkBVLLPoldRPzyGySYqEo3vPYUfc
9UIbi+h+DcSQBVNzZ8aF0pGdtiupC55I43tD6qZ6qCfERp1/I/OHw/t1vf5r2IsE
doFkG6FnUffl7alFXcJkO6XK+EVV9pYUSGFDzTCtyZyl6JTK6KGJ2F5E9ftW4amn
Y9Wxyx+/vEG1S1zNdJMWwpBhSKHradCh3sUoNH3vfD+HrTZX2ucVyzfQwTmEme1J
vENfUGq+jhI8ihb7ol9FNXriCfUumT+lv147WA8z5QEk8xm9lo17HcvGA8iedEVm
i6y4Fmw3rTNhJSkEX/nCQCFaIz2Rz+rvaKHHSmzYSkVkOeJjJgLegPYRjeKx1Htb
jGsDU1SjmQ1n0ve1CYTXaQ6yMdg5iqLDt6VHSERcfy+kX0kgib4=
=Sb7C
-----END PGP SIGNATURE-----

--F/pwlVaeN43cP1D5--