8000 Fix subtly-incorrect matching of parent and child partitioned indexes. · postgres/postgres@116f20f · GitHub
[go: up one dir, main page]
Skip to content

Commit 116f20f

Browse files
tglsfdctglsfdc
committed
Fix subtly-incorrect matching of parent and child partitioned indexes.
When creating a partitioned index, DefineIndex tries to identify any existing indexes on the partitions that match the partitioned index, so that it can absorb those as child indexes instead of building new ones. Part of the matching is to compare IndexInfo structs --- but that wasn't done quite right. We're comparing the IndexInfo built within DefineIndex itself to one made from existing catalog contents by BuildIndexInfo. Notably, while BuildIndexInfo will run index expressions and predicates through expression preprocessing, that has not happened to DefineIndex's struct. The result is failure to match and subsequent creation of duplicate indexes. The easiest and most bulletproof fix is to build a new IndexInfo using BuildIndexInfo, thereby guaranteeing that the processing done is identical. While here, let's also extract the opfamily and collation data from the new partitioned index, removing ad-hoc logic that duplicated knowledge about how those are constructed. Per report from Christophe Pettus. Back-patch to v11 where we invented partitioned indexes. Richard Guo and Tom Lane Discussion: https://postgr.es/m/8864BFAA-81FD-4BF9-8E06-7DEB8D4164ED@thebuild.com
1 parent 9d4c9c3 commit 116f20f

File tree

3 files changed

+96
-8
lines changed

3 files changed

+96
-8
lines changed

‎src/backend/commands/indexcmds.c

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,15 +1009,24 @@ DefineIndex(Oid relationId,
10091009
int nparts = partdesc->nparts;
10101010
Oid *part_oids = palloc(sizeof(Oid) * nparts);
10111011
bool invalidate_parent = false;
1012+
Relation parentIndex;
10121013
TupleDesc parentDesc;
1013-
Oid *opfamOids;
10141014

1015+
/* Make a local copy of partdesc->oids[], just for safety */
10151016
memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
10161017

1018+
/*
1019+
* We'll need an IndexInfo describing the parent index. The one
1020+
* built above is almost good enough, but not quite, because (for
1021+
* example) its predicate expression if any hasn't been through
1022+
* expression preprocessing. The most reliable way to get an
1023+
* IndexInfo that will match those for child indexes is to build
1024+
* it the same way, using BuildIndexInfo().
1025+
*/
1026+
parentIndex = index_open(indexRelationId, lockmode);
1027+
indexInfo = BuildIndexInfo(parentIndex);
1028+
10171029
parentDesc = RelationGetDescr(rel);
1018-
opfamOids = palloc(sizeof(Oid) * numberOfKeyAttributes);
1019-
for (i = 0; i < numberOfKeyAttributes; i++)
1020-
opfamOids[i] = get_opclass_family(classObjectId[i]);
10211030

10221031
/*
10231032
* For each partition, scan all existing indexes; if one matches
@@ -1091,9 +1100,9 @@ DefineIndex(Oid relationId,
10911100
cldIdxInfo = BuildIndexInfo(cldidx);
10921101
if (CompareIndexInfo(cldIdxInfo, indexInfo,
10931102
cldidx->rd_indcollation,
1094-
collationObjectId,
1103+
parentIndex->rd_indcollation,
10951104
cldidx->rd_opfamily,
1096-
opfamOids,
1105+
parentIndex->rd_opfamily,
10971106
attmap, maplen))
10981107
{
10991108
Oid cldConstrOid = InvalidOid;
@@ -1216,6 +1225,8 @@ DefineIndex(Oid relationId,
12161225
pfree(attmap);
12171226
}
12181227

1228+
index_close(parentIndex, lockmode);
1229+
12191230
/*
12201231
* The pg_index row we inserted for this index was marked
12211232
* indisvalid=true. But if we attached an existing index that is

‎src/test/regress/expected/indexing.out

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ drop table idxpart;
349349
-- When a table is attached a partition and it already has an index, a
350350
-- duplicate index should not get created, but rather the index becomes
351351
-- attached to the parent's index.
352-
create table idxpart (a int, b int, c text) partition by range (a);
352+
create table idxpart (a int, b int, c text, d bool) partition by range (a);
353353
create index idxparti on idxpart (a);
354354
create index idxparti2 on idxpart (b, c);
355355
create table idxpart1 (like idxpart including indexes);
@@ -360,6 +360,7 @@ create table idxpart1 (like idxpart including indexes);
360360
a | integer | | |
361361
b | integer | | |
362362
c | text | | |
363+
d | boolean | | |
363364
Indexes:
364365
"idxpart1_a_idx" btree (a)
365366
"idxpart1_b_c_idx" btree (b, c)
@@ -386,6 +387,7 @@ alter table idxpart attach partition idxpart1 for values from (0) to (10);
386387
a | integer | | |
387388
b | integer | | |
388389
c | text | | |
390+
d | boolean | | |
389391
Partition of: idxpart FOR VALUES FROM (0) TO (10)
390392
Indexes:
391393
"idxpart1_a_idx" btree (a)
@@ -405,6 +407,68 @@ select relname, relkind, inhparent::regclass
405407
idxparti2 | I |
406408
(6 rows)
407409

410+
-- While here, also check matching when creating an index after the fact.
411+
create index on idxpart1 ((a+b)) where d = true;
412+
\d idxpart1
413+
Table "public.idxpart1"
414+
Column | Type | Collation | Nullable | Default
415+
--------+---------+-----------+----------+---------
416+
a | integer | | |
417+
b | integer | | |
418+
c | text | | |
419+
d | boolean | | |
420+
Partition of: idxpart FOR VALUES FROM (0) TO (10)
421+
Indexes:
422+
"idxpart1_a_idx" btree (a)
423+
"idxpart1_b_c_idx" btree (b, c)
424+
"idxpart1_expr_idx" btree ((a + b)) WHERE d = true
425+
426+
select relname, relkind, inhparent::regclass
427+
from pg_c 67E6 lass left join pg_index ix on (indexrelid = oid)
428+
left join pg_inherits on (ix.indexrelid = inhrelid)
429+
where relname like 'idxpart%' order by relname;
430+
relname | relkind | inhparent
431+
-------------------+---------+-----------
432+
idxpart | p |
433+
idxpart1 | r |
434+
idxpart1_a_idx | i | idxparti
435+
idxpart1_b_c_idx | i | idxparti2
436+
idxpart1_expr_idx | i |
437+
idxparti | I |
438+
idxparti2 | I |
439+
(7 rows)
440+
441+
create index idxparti3 on idxpart ((a+b)) where d = true;
442+
\d idxpart1
443+
Table "public.idxpart1"
444+
Column | Type | Collation | Nullable | Default
445+
--------+---------+-----------+----------+---------
446+
a | integer | | |
447+
b | integer | | |
448+
c | text | | |
449+
d | boolean | | |
450+
Partition of: idxpart FOR VALUES FROM (0) TO (10)
451+
Indexes:
452+
"idxpart1_a_idx" btree (a)
453+
"idxpart1_b_c_idx" btree (b, c)
454+
"idxpart1_expr_idx" btree ((a + b)) WHERE d = true
455+
456+
select relname, relkind, inhparent::regclass
457+
from pg_class left join pg_index ix on (indexrelid = oid)
458+
left join pg_inherits on (ix.indexrelid = inhrelid)
459+
where relname like 'idxpart%' order by relname;
460+
relname | relkind | inhparent
461+
-------------------+---------+-----------
462+
idxpart | p |
463+
idxpart1 | r |
464+
idxpart1_a_idx | i | idxparti
465+
idxpart1_b_c_idx | i | idxparti2
466+
idxpart1_expr_idx | i | idxparti3
467+
idxparti | I |
468+
idxparti2 | I |
469+
idxparti3 | I |
470+
(8 rows)
471+
408472
drop table idxpart;
409473
-- Verify that attaching an invalid index does not mark the parent index valid.
410474
-- On the other hand, attaching a valid index marks not only its direct

‎src/test/regress/sql/indexing.sql

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ drop table idxpart;
175175
-- When a table is attached a partition and it already has an index, a
176176
-- duplicate index should not get created, but rather the index becomes
177177
-- attached to the parent's index.
178-
create table idxpart (a int, b int, c text) partition by range (a);
178+
create table idxpart (a int, b int, c text, d bool) partition by range (a);
179179
create index idxparti on idxpart (a);
180180
create index idxparti2 on idxpart (b, c);
181181
create table idxpart1 (like idxpart including indexes);
@@ -186,6 +186,19 @@ select relname, relkind, inhparent::regclass
186186
where relname like 'idxpart%' order by relname;
187187
alter table idxpart attach partition idxpart1 for values from (0) to (10);
188188
\d idxpart1
189+
select relname, relkind, inhparent::regclass
190+
from pg_class left join pg_index ix on (indexrelid = oid)
191+
left join pg_inherits on (ix.indexrelid = inhrelid)
192+
where relname like 'idxpart%' order by relname;
193+
-- While here, also check matching when creating an index after the fact.
194+
create index on idxpart1 ((a+b)) where d = true;
195+
\d idxpart1
196+
select relname, relkind, inhparent::regclass
197+
from pg_class left join pg_index ix on (indexrelid = oid)
198+
left join pg_inherits on (ix.indexrelid = inhrelid)
199+
where relname like 'idxpart%' order by relname;
200+
create index idxparti3 on idxpart ((a+b)) where d = true;
201+
\d idxpart1
189202
select relname, relkind, inhparent::regclass
190203
from pg_class left join pg_index ix on (indexrelid = oid)
191204
left join pg_inherits on (ix.indexrelid = inhrelid)

0 commit comments

Comments
 (0)
0