Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e6dbb48

Browse files
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 3e63e84 commit e6dbb48

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
@@ -1213,18 +1213,27 @@ DefineIndex(Oid relationId,
12131213
int nparts = partdesc->nparts;
12141214
Oid *part_oids = palloc(sizeof(Oid) * nparts);
12151215
bool invalidate_parent = false;
1216+
Relation parentIndex;
12161217
TupleDesc parentDesc;
1217-
Oid *opfamOids;
12181218

12191219
pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
12201220
nparts);
12211221

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

1225+
/*
1226+
* We'll need an IndexInfo describing the parent index. The one
1227+
* built above is almost good enough, but not quite, because (for
1228+
* example) its predicate expression if any hasn't been through
1229+
* expression preprocessing. The most reliable way to get an
1230+
* IndexInfo that will match those for child indexes is to build
1231+
* it the same way, using BuildIndexInfo().
1232+
*/
1233+
parentIndex = index_open(indexRelationId, lockmode);
1234+
indexInfo = BuildIndexInfo(parentIndex);
1235+
12241236
parentDesc = RelationGetDescr(rel);
1225-
opfamOids = palloc(sizeof(Oid) * numberOfKeyAttributes);
1226-
for (i = 0; i < numberOfKeyAttributes; i++)
1227-
opfamOids[i] = get_opclass_family(classObjectId[i]);
12281237

12291238
/*
12301239
* For each partition, scan all existing indexes; if one matches
@@ -1295,9 +1304,9 @@ DefineIndex(Oid relationId,
12951304
cldIdxInfo = BuildIndexInfo(cldidx);
12961305
if (CompareIndexInfo(cldIdxInfo, indexInfo,
12971306
cldidx->rd_indcollation,
1298-
collationObjectId,
1307+
parentIndex->rd_indcollation,
12991308
cldidx->rd_opfamily,
1300-
opfamOids,
1309+
parentIndex->rd_opfamily,
13011310
attmap))
13021311
{
13031312
Oid cldConstrOid = InvalidOid;
@@ -1424,6 +1433,8 @@ DefineIndex(Oid relationId,
14241433
free_attrmap(attmap);
14251434
}
14261435

1436+
index_close(parentIndex, lockmode);
1437+
14271438
/*
14281439
* The pg_index row we inserted for this index was marked
14291440
* 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
@@ -378,7 +378,7 @@ drop table idxpart;
378378
-- When a table is attached a partition and it already has an index, a
379379
-- duplicate index should not get created, but rather the index becomes
380380
-- attached to the parent's index.
381-
create table idxpart (a int, b int, c text) partition by range (a);
381+
create table idxpart (a int, b int, c text, d bool) partition by range (a);
382382
create index idxparti on idxpart (a);
383383
create index idxparti2 on idxpart (b, c);
384384
create table idxpart1 (like idxpart including indexes);
@@ -389,6 +389,7 @@ create table idxpart1 (like idxpart including indexes);
389389
a | integer | | |
390390
b | integer | | |
391391
c | text | | |
392+
d | boolean | | |
392393
Indexes:
393394
"idxpart1_a_idx" btree (a)
394395
"idxpart1_b_c_idx" btree (b, c)
@@ -415,6 +416,7 @@ alter table idxpart attach partition idxpart1 for values from (0) to (10);
415416
a | integer | | |
416417
b | integer | | |
417418
c | text | | |
419+
d | boolean | | |
418420
Partition of: idxpart FOR VALUES FROM (0) TO (10)
419421
Indexes:
420422
"idxpart1_a_idx" btree (a)
@@ -434,6 +436,68 @@ select relname, relkind, inhparent::regclass
434436
idxparti2 | I |
435437
(6 rows)
436438

439+
-- While here, also check matching when creating an index after the fact.
440+
create index on idxpart1 ((a+b)) where d = true;
441+
\d idxpart1
442+
Table "public.idxpart1"
443+
Column | Type | Collation | Nullable | Default
444+
--------+---------+-----------+----------+---------
445+
a | integer | | |
446+
b | integer | | |
447+
c | text | | |
448+
d | boolean | | |
449+
Partition of: idxpart FOR VALUES FROM (0) TO (10)
450+
Indexes:
451+
"idxpart1_a_idx" btree (a)
452+
"idxpart1_b_c_idx" btree (b, c)
453+
"idxpart1_expr_idx" btree ((a + b)) WHERE d = true
454+
455+
select relname, relkind, inhparent::regclass
456+
from pg_class left join pg_index ix on (indexrelid = oid)
457+
left join pg_inherits on (ix.indexrelid = inhrelid)
458+
where relname like 'idxpart%' order by relname;
459+
relname | relkind | inhparent
460+
-------------------+---------+-----------
461+
idxpart | p |
462+
idxpart1 | r |
463+
idxpart1_a_idx | i | idxparti
464+
idxpart1_b_c_idx | i | idxparti2
465+
idxpart1_expr_idx | i |
466+
idxparti | I |
467+
idxparti2 | I |
468+
(7 rows)
469+
470+
create index idxparti3 on idxpart ((a+b)) where d = true;
471+
\d idxpart1
472+
Table "public.idxpart1"
473+
Column | Type | Collation | Nullable | Default
474+
--------+---------+-----------+----------+---------
475+
a | integer | | |
476+
b | integer | | |
477+
c | text | | |
478+
d | boolean | | |
479+
Partition of: idxpart FOR VALUES FROM (0) TO (10)
480+
Indexes:
481+
"idxpart1_a_idx" btree (a)
482+
"idxpart1_b_c_idx" btree (b, c)
483+
"idxpart1_expr_idx" btree ((a + b)) WHERE d = true
484+
485+
select relname, relkind, inhparent::regclass
486+
from pg_class left join pg_index ix on (indexrelid = oid)
487+
left join pg_inherits on (ix.indexrelid = inhrelid)
488+
where relname like 'idxpart%' order by relname;
489+
relname | relkind | inhparent
490+
-------------------+---------+-----------
491+
idxpart | p |
492+
idxpart1 | r |
493+
idxpart1_a_idx | i | idxparti
494+
idxpart1_b_c_idx | i | idxparti2
495+
idxpart1_expr_idx | i | idxparti3
496+
idxparti | I |
497+
idxparti2 | I |
498+
idxparti3 | I |
499+
(8 rows)
500+
437501
drop table idxpart;
438502
-- Verify that attaching an invalid index does not mark the parent index valid.
439503
-- 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
@@ -192,7 +192,7 @@ drop table idxpart;
192192
-- When a table is attached a partition and it already has an index, a
193193
-- duplicate index should not get created, but rather the index becomes
194194
-- attached to the parent's index.
195-
create table idxpart (a int, b int, c text) partition by range (a);
195+
create table idxpart (a int, b int, c text, d bool) partition by range (a);
196196
create index idxparti on idxpart (a);
197197
create index idxparti2 on idxpart (b, c);
198198
create table idxpart1 (like idxpart including indexes);
@@ -203,6 +203,19 @@ select relname, relkind, inhparent::regclass
203203
where relname like 'idxpart%' order by relname;
204204
alter table idxpart attach partition idxpart1 for values from (0) to (10);
205205
\d idxpart1
206+
select relname, relkind, inhparent::regclass
207+
from pg_class left join pg_index ix on (indexrelid = oid)
208+
left join pg_inherits on (ix.indexrelid = inhrelid)
209+
where relname like 'idxpart%' order by relname;
210+
-- While here, also check matching when creating an index after the fact.
211+
create index on idxpart1 ((a+b)) where d = true;
212+
\d idxpart1
213+
select relname, relkind, inhparent::regclass
214+
from pg_class left join pg_index ix on (indexrelid = oid)
215+
left join pg_inherits on (ix.indexrelid = inhrelid)
216+
where relname like 'idxpart%' order by relname;
217+
create index idxparti3 on idxpart ((a+b)) where d = true;
218+
\d idxpart1
206219
select relname, relkind, inhparent::regclass
207220
from pg_class left join pg_index ix on (indexrelid = oid)
208221
left join pg_inherits on (ix.indexrelid = inhrelid)

0 commit comments

Comments
 (0)