| From: | Sami Imseih <samimseih(at)gmail(dot)com> |
|---|---|
| To: | Alberto Piai <alberto(dot)piai(at)gmail(dot)com> |
| Cc: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Xuneng Zhou <xunengzhou(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz> |
| Subject: | Re: tablecmds: fix bug where index rebuild loses replica identity on partitions |
| Date: | 2026-07-27 22:10:29 |
| Message-ID: | CAA5RZ0s6P1q38rLdBbKCB6X4vXs-2uuUZOARkc03b-K1-uHQag@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
I spent some time looking at this, and I think there are still some
issues with v9.
For example:
```
CREATE TABLE p (id int NOT NULL, val int NOT NULL)
PARTITION BY RANGE (id);
CREATE TABLE c1 (id int NOT NULL, val int NOT NULL);
CREATE UNIQUE INDEX my_custom_ri ON c1 (id, val);
ALTER TABLE c1 REPLICA IDENTITY USING INDEX my_custom_ri;
CREATE UNIQUE INDEX p_idx ON p (id, val);
ALTER TABLE p ATTACH PARTITION c1
FOR VALUES FROM (0) TO (100);
ALTER TABLE p ALTER COLUMN val TYPE bigint;
ERROR: index "my_custom_ri" for table "c1" does not exist
```
This is because generateClonedIndexStmt() sets idxname = NULL when building
the partition indexes.
```
IndexStmt *
generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx, ...
/*
* We don't try to preserve the name of the source index; instead,
* just let DefineIndex() choose a reasonable name. (If we tried to
* preserve the name, we'd get duplicate-relation-name failures
* unless the source table was in a different schema.)
*/
index->idxname = NULL;
```
This causes ChooseIndexName() to generate a new default name. v9 then
tries to restore replica identity by the old name, which no longer
exists.
This index name-change behavior during ALTER TABLE ... ALTER COLUMN
TYPE is existing and undocumented, and I would argue is wrong. The
user expects index names to be stable at the end of this operation.
CREATE TABLE ... LIKE (INCLUDING INDEXES) is a case where we should not
create an index with the same name as the source and choosing a default name
makes sense, but it should not apply to all callers.
Perhaps callers like ALTER TABLE ... ALTER COLUMN TYPE should track
the old index names and be allowed to pass them to generateClonedIndexStmt?
That sounds like a broader improvement, and one that the changes in v9
can inherit.
What do you think?
The current v9 tests happen to work only because they use default names that
ChooseIndexName() re-generates the same way.
Also, this issue is not limited to replica identity. CLUSTER ON does
not copy/restore indisclustered on partition indexes at all, and this
should be addressed as well, right?
--
Sami Imseih
Amazon Web Services (AWS)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Sami Imseih | 2026-07-27 22:21:13 | Re: Track skipped tables during autovacuum and autoanalyze |
| Previous Message | Corey Huinker | 2026-07-27 21:07:32 | Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions |