Thread: ALTER TABLE .. ADD PRIMARY KEY .. USING INDEX has dump-restore hazard
Consider: rhaas=# create table foo (a text); CREATE TABLE rhaas=# create unique index on foo (a collate "C"); CREATE INDEX rhaas=# alter table foo add primary key using index foo_a_idx; ALTER TABLE rhaas=# \d foo Table "public.foo"Column | Type | Modifiers --------+------+-----------a | text | not null Indexes: "foo_a_idx" PRIMARY KEY, btree (a COLLATE "C") Now dump and restore this database. Then: rhaas=# \d foo Table "public.foo"Column | Type | Modifiers --------+------+-----------a | text | not null Indexes: "foo_a_idx" PRIMARY KEY, btree (a) Notice that the collation specifier is gone. Oops. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: ALTER TABLE .. ADD PRIMARY KEY .. USING INDEX has dump-restore hazard
From
Michael Paquier
Date:
On Wed, Jul 22, 2015 at 1:23 AM, Robert Haas <robertmhaas@gmail.com> wrote: > Notice that the collation specifier is gone. Oops. As it is not possible to specify directly a constraint for a PRIMARY KEY expression, what about switching dumpConstraint to have it use first a CREATE INDEX query with the collation and then use ALTER TABLE to attach the constraint to it? I am noticing that we already fetch the index definition in indxinfo via pg_get_indexdef. Thoughts? -- Michael
Re: ALTER TABLE .. ADD PRIMARY KEY .. USING INDEX has dump-restore hazard
From
Michael Paquier
Date:
On Wed, Jul 22, 2015 at 9:34 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Wed, Jul 22, 2015 at 1:23 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> Notice that the collation specifier is gone. Oops. > > As it is not possible to specify directly a constraint for a PRIMARY > KEY expression, what about switching dumpConstraint to have it use > first a CREATE INDEX query with the collation and then use ALTER TABLE > to attach the constraint to it? I am noticing that we already fetch > the index definition in indxinfo via pg_get_indexdef. Thoughts? And poking at that I have finished with the attached that adds a CREATE INDEX query before ALTER TABLE ADD CONSTRAINT, to which a USING INDEX is appended. Storage options as well as building the column list becomes unnecessary because indexdef already provides everything what is needed, so this patch makes dump rely more on what is on backend-side. -- Michael
Attachment
On Tue, Jul 21, 2015 at 8:34 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Wed, Jul 22, 2015 at 1:23 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> Notice that the collation specifier is gone. Oops. > > As it is not possible to specify directly a constraint for a PRIMARY > KEY expression, what about switching dumpConstraint to have it use > first a CREATE INDEX query with the collation and then use ALTER TABLE > to attach the constraint to it? I am noticing that we already fetch > the index definition in indxinfo via pg_get_indexdef. Thoughts? I guess the questions on my mind is "Why is it that you can't do this directly when creating a primary key, but you can do it when turning an existing index into a primary key?" If there's a good reason for prohibiting doing this when creating a primary key, then presumably it shouldn't be allowed when turning an index into a primary key either. If there's not, then why not extend the PRIMARY KEY syntax to allow it? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Jul 22, 2015 at 7:34 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Jul 21, 2015 at 8:34 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Wed, Jul 22, 2015 at 1:23 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> Notice that the collation specifier is gone. Oops.
>
> As it is not possible to specify directly a constraint for a PRIMARY
> KEY expression, what about switching dumpConstraint to have it use
> first a CREATE INDEX query with the collation and then use ALTER TABLE
> to attach the constraint to it? I am noticing that we already fetch
> the index definition in indxinfo via pg_get_indexdef. Thoughts?
I guess the questions on my mind is "Why is it that you can't do this
directly when creating a primary key, but you can do it when turning
an existing index into a primary key?"
If there's a good reason for prohibiting doing this when creating a
primary key, then presumably it shouldn't be allowed when turning an
index into a primary key either. If there's not, then why not extend
the PRIMARY KEY syntax to allow it?
+1. I think in the short term we can treat this as a bug, and "fix" the bug by disallowing an index with attributes that cannot be present in an index created by PRIMARY KEY constraint. The collation attribute on one of the keys may be just one of many such attributes.
In the long term, we may want to allow collation in primary key, but that will be a feature ideally suited for a major version release.
Best regards,
-- Gurjeet Singh http://gurjeet.singh.im/
On Tue, Jul 21, 2015 at 9:23 AM, Robert Haas <robertmhaas@gmail.com> wrote:
rhaas=# create unique index on foo (a collate "C");
CREATE INDEX
rhaas=# alter table foo add primary key using index foo_a_idx;
ALTER TABLE
Now dump and restore this database. Then:
Notice that the collation specifier is gone. Oops.
The intent of the 'USING INDEX' feature was to work around the problem that PRIMARY KEY indexes cannot be reindexed concurrently to reduce bloat.
Considering that the feature doesn't work [1] (at least as shown in example in the docs [2]) if there are any foreign keys referencing the table, there's scope for improvement.
There was proposal to support reindexing the primary key index, but I am not sure where that stands. If that's already in, or soon to be in core, we may put a deprecation notice around USING INDEX. OTOH, if reindexing PKeys is too difficult, we may want to support index-replacement via a top-level ALTER TABLE subcommand that works in CONCURRENT mode, and not recommend the method shown in the docs (i.e deprecate the USING INDEX syntax).
[1]: The DROP CONSTRAINT part of the command fails if there are any FKeys pointing to this table.
[2]: http://www.postgresql.org/docs/9.4/static/sql-altertable.htmlGurjeet Singh http://gurjeet.singh.im/
Re: ALTER TABLE .. ADD PRIMARY KEY .. USING INDEX has dump-restore hazard
From
Michael Paquier
Date:
On Thu, Jul 23, 2015 at 5:47 AM, Gurjeet Singh <gurjeet@singh.im> wrote: > On Wed, Jul 22, 2015 at 7:34 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> >> On Tue, Jul 21, 2015 at 8:34 PM, Michael Paquier >> <michael.paquier@gmail.com> wrote: >> > On Wed, Jul 22, 2015 at 1:23 AM, Robert Haas <robertmhaas@gmail.com> >> > wrote: >> >> Notice that the collation specifier is gone. Oops. >> > >> > As it is not possible to specify directly a constraint for a PRIMARY >> > KEY expression, what about switching dumpConstraint to have it use >> > first a CREATE INDEX query with the collation and then use ALTER TABLE >> > to attach the constraint to it? I am noticing that we already fetch >> > the index definition in indxinfo via pg_get_indexdef. Thoughts? >> >> I guess the questions on my mind is "Why is it that you can't do this >> directly when creating a primary key, but you can do it when turning >> an existing index into a primary key?" Sure. This does not seem to be complicated. >> If there's a good reason for prohibiting doing this when creating a >> primary key, then presumably it shouldn't be allowed when turning an >> index into a primary key either. If there's not, then why not extend >> the PRIMARY KEY syntax to allow it? Yeah, I think we should be able to define a collation in this case. For example it is as well possible to pass a WITH clause with storage parameters, though we do not document it in table_constraint_using_index (http://www.postgresql.org/docs/devel/static/sql-altertable.html). > +1. I think in the short term we can treat this as a bug, and "fix" the bug > by disallowing an index with attributes that cannot be present in an index > created by PRIMARY KEY constraint. The collation attribute on one of the > keys may be just one of many such attributes. Er, pushing such a patch on back-branches may break applications that do exactly what Robert did in his test case (using a unique index as primary key with a collation), no? I am not sure that this is acceptable. Taking the problem at its root by swtiching pg_dump to define an index and then use it looks a more solid approach on back branches. > In the long term, we may want to allow collation in primary key, but that > will be a feature ideally suited for a major version release. Yep. Definitely. -- Michael
Re: ALTER TABLE .. ADD PRIMARY KEY .. USING INDEX has dump-restore hazard
From
Michael Paquier
Date:
On Thu, Jul 23, 2015 at 9:06 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > Yeah, I think we should be able to define a collation in this case. > For example it is as well possible to pass a WITH clause with storage > parameters, though we do not document it in > table_constraint_using_index > (http://www.postgresql.org/docs/devel/static/sql-altertable.html). Mistake here, please ignore this remark. USING INDEX does not accept WITH storage_parameter but ADD CONSTRAINT without an index does and it is documented in CREATE TABLE. -- Michael