From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: exclusion@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17997: Assert failed in validatePartitionedIndex() when attaching partition index to child of valid index
Date: Mon, 26 Jun 2023 17:05:37 +0900
Message-ID: <ZJlG0a/F03iPI/wj@paquier.xyz> (raw)
In-Reply-To: <ZJk04dp0mz7s58Lx@paquier.xyz>
References: <17997-a044c27aef95daf8@postgresql.org>
<509915.1687704058@sss.pgh.pa.us>
<ZJk04dp0mz7s58Lx@paquier.xyz>
On Mon, Jun 26, 2023 at 03:49:05PM +0900, Michael Paquier wrote:
> It seems to me that there are two potential solutions here:
> 1) We could recurse and make sure that the index of t *also* becomes
> *invalid*, because t_a_idx and tp1_a_idx form a partition chain, still
> the leaf tp1_1 does *not* have an index yet, making it unusable.
> 2) Then I found about AttachPartitionEnsureIndexes(). This is called
> when attaching a partition to make sure that indexes are created to
> match the indexes between the partitioned table and the new partition
> attached to it, but it does not recurse when attaching a partitioned
> table, or an index would have been created on tp1_1 when running the
> 6th command to complete the index tree all the way down.
>
> At the end, I'd like to think that 2) is the right path forward. For
> example, this creates a partitioned index automatically when attaching
> a partition:
> create table t(a int) partition by range (a);
> create index on t(a);
> create table tp1(a int) partition by range (a);
> alter table t attach partition tp1 for values from (1) to (100);
> =# \di
> List of relations
> Schema | Name | Type | Owner | Table
> --------+-----------+-------------------+--------+-------
> public | t_a_idx | partitioned index | popopo | t
> public | tp1_a_idx | partitioned index | popopo | tp1
> (2 rows)
A third solution that came into my mind just now would be to revisit
the choice done in AttachPartitionEnsureIndexes() where an invalid
index can be chosen as a match when creating the indexes on the
partitions, so as we are able to get to the bottom of a chain with
valid indexes for the whole tree. I have been testing the attached
and it has been working here the way I'd expect when manipulating
partition trees with ATTACH PARTITION, though this breaks the scenario
of this bug report because we would now get a failure when attempting
to attach an index in the last command.
Thoughts or comments?
--
Michael
Attachments:
[text/x-diff] 0001-Fix-ATTACH-PARTITION-to-ignore-invalid-indexes-as-ma.patch (5.0K, ../ZJlG0a%2FF03iPI%2Fwj@paquier.xyz/2-0001-Fix-ATTACH-PARTITION-to-ignore-invalid-indexes-as-ma.patch)
download | inline diff:
From 3359ecbc304fecca924c637d56c105ef7e5a6497 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 26 Jun 2023 17:05:02 +0900
Subject: [PATCH] Fix ATTACH PARTITION to ignore invalid indexes as match
---
src/backend/commands/tablecmds.c | 8 +++--
src/test/regress/expected/partition_info.out | 36 ++++++++++++++++++++
src/test/regress/sql/partition_info.sql | 20 +++++++++++
3 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 9b12bc44d7..d985278ac6 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -18104,8 +18104,8 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
/*
* Scan the list of existing indexes in the partition-to-be, and mark
- * the first matching, unattached one we find, if any, as partition of- * the parent index. If we find one, we're done.+ * the first matching, valid, unattached one we find, if any, as+ * partition of the parent index. If we find one, we're done.
*/
for (i = 0; i < list_length(attachRelIdxs); i++)
{
@@ -18116,6 +18116,10 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
if (attachrelIdxRels[i]->rd_rel->relispartition)
continue;
+ /* If this index is invalid, can't use it */+ if (!attachrelIdxRels[i]->rd_index->indisvalid)+ continue;+
if (CompareIndexInfo(attachInfos[i], info,
attachrelIdxRels[i]->rd_indcollation,
idxRel->rd_indcollation,
diff --git a/src/test/regress/expected/partition_info.out b/src/test/regress/expected/partition_info.outindex 42b6bc77ca..4ee51a2f73 100644--- a/src/test/regress/expected/partition_info.out+++ b/src/test/regress/expected/partition_info.out@@ -349,3 +349,39 @@ SELECT pg_partition_root('ptif_li_child');
DROP VIEW ptif_test_view;
DROP MATERIALIZED VIEW ptif_test_matview;
DROP TABLE ptif_li_parent, ptif_li_child;
+-- Check that invalid indexes are not selected when attaching a partition.+CREATE TABLE ptif_inval_tab (a int) PARTITION BY RANGE (a);+CREATE INDEX ptif_inval_idx ON ptif_inval_tab (a);+CREATE TABLE ptif_inval_tab_1 (a int) PARTITION BY RANGE (a);+CREATE TABLE ptif_inval_tab_1_1 PARTITION OF ptif_inval_tab_1+ FOR VALUES FROM (0) TO (10);+CREATE TABLE ptif_inval_tab_1_2 PARTITION OF ptif_inval_tab_1+ FOR VALUES FROM (10) TO (20);+-- This creates an invalid index.+CREATE INDEX ptif_inval_ixd_1 ON ONLY ptif_inval_tab_1 (a);+-- This creates new indexes for all the partitions of ptif_inval_tab_1,+-- discarding the invalid index created previously as something to choose.+ALTER TABLE ptif_inval_tab ATTACH PARTITION ptif_inval_tab_1+ FOR VALUES FROM (1) to (100);+SELECT indexrelid::regclass, indisvalid FROM pg_index+ WHERE indexrelid::regclass::text ~ 'ptif_inval_'+ ORDER BY indexrelid::regclass::text;+ indexrelid | indisvalid +--------------------------+------------+ ptif_inval_idx | t+ ptif_inval_ixd_1 | f+ ptif_inval_tab_1_1_a_idx | t+ ptif_inval_tab_1_2_a_idx | t+ ptif_inval_tab_1_a_idx | t+(5 rows)++SELECT * FROM pg_partition_tree('ptif_inval_idx');+ relid | parentrelid | isleaf | level +--------------------------+------------------------+--------+-------+ ptif_inval_idx | | f | 0+ ptif_inval_tab_1_a_idx | ptif_inval_idx | f | 1+ ptif_inval_tab_1_1_a_idx | ptif_inval_tab_1_a_idx | t | 2+ ptif_inval_tab_1_2_a_idx | ptif_inval_tab_1_a_idx | t | 2+(4 rows)++DROP TABLE ptif_inval_tab;diff --git a/src/test/regress/sql/partition_info.sql b/src/test/regress/sql/partition_info.sqlindex b5060bec7f..5d5f142728 100644--- a/src/test/regress/sql/partition_info.sql+++ b/src/test/regress/sql/partition_info.sql@@ -127,3 +127,23 @@ SELECT pg_partition_root('ptif_li_child');
DROP VIEW ptif_test_view;
DROP MATERIALIZED VIEW ptif_test_matview;
DROP TABLE ptif_li_parent, ptif_li_child;
++-- Check that invalid indexes are not selected when attaching a partition.+CREATE TABLE ptif_inval_tab (a int) PARTITION BY RANGE (a);+CREATE INDEX ptif_inval_idx ON ptif_inval_tab (a);+CREATE TABLE ptif_inval_tab_1 (a int) PARTITION BY RANGE (a);+CREATE TABLE ptif_inval_tab_1_1 PARTITION OF ptif_inval_tab_1+ FOR VALUES FROM (0) TO (10);+CREATE TABLE ptif_inval_tab_1_2 PARTITION OF ptif_inval_tab_1+ FOR VALUES FROM (10) TO (20);+-- This creates an invalid index.+CREATE INDEX ptif_inval_ixd_1 ON ONLY ptif_inval_tab_1 (a);+-- This creates new indexes for all the partitions of ptif_inval_tab_1,+-- discarding the invalid index created previously as something to choose.+ALTER TABLE ptif_inval_tab ATTACH PARTITION ptif_inval_tab_1+ FOR VALUES FROM (1) to (100);+SELECT indexrelid::regclass, indisvalid FROM pg_index+ WHERE indexrelid::regclass::text ~ 'ptif_inval_'+ ORDER BY indexrelid::regclass::text;+SELECT * FROM pg_partition_tree('ptif_inval_idx');+DROP TABLE ptif_inval_tab;--
2.40.1
[application/pgp-signature] signature.asc (832B, ../ZJlG0a%2FF03iPI%2Fwj@paquier.xyz/3-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, tgl@sss.pgh.pa.us, exclusion@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17997: Assert failed in validatePartitionedIndex() when attaching partition index to child of valid index
In-Reply-To: <ZJlG0a/F03iPI/wj@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox