Setting a pre-existing index as a primary key

Hey all,

I've run into a couple cases now where it would be helpful to easily
assign an already-existing unique index as a primary key. Unless I
completely missed something, there's no way to do this now without a
bit of catalog hackery.

My implementation idea is as follows:

Proposed Syntax (based on Oracle's syntax)

ALTER TABLE foo ADD CONSTRAINT bar PRIMARY KEY USING INDEX schema.tablename;

Proposed Implementation

1. Verify that the index named is a unique index
2. Check index columns for NOT NULL constraints
3. If indexed columns are not already NOT NULL, apply NOT NULL
4. If NOT NULL succeeds, complete the operation (catalogs,
dependencies, ...), else bail out.

Any comments, ideas, suggestions?

--
Jonah H. Harris, Sr. Software Architect | phone: 732.331.1324
EnterpriseDB Corporation | fax: 732.331.1301
499 Thornall Street, 2nd Floor | jonah(dot)harris(at)enterprisedb(dot)com
Edison, NJ 08837 | http://www.enterprisedb.com/

"Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> I've run into a couple cases now where it would be helpful to easily
> assign an already-existing unique index as a primary key.

You need to present a more convincing use-case than this unsupported
assertion. There's hardly any effective difference between a unique
index + NOT NULL constraints and a declared primary key ... so what
did you really need it for?

> 1. Verify that the index named is a unique index

... and not partial, and not on expressions, and not invalid, and not
using non-default opclasses (which might have a surprising definition of
"equal"), and not already owned by a constraint ... not to mention that
it'd better be an index on the named table, which among other things
removes the need for a schema specification on the index name.

regards, tom lane

On Tue, Apr 8, 2008 at 9:04 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> > I've run into a couple cases now where it would be helpful to easily
> > assign an already-existing unique index as a primary key.
>
> You need to present a more convincing use-case than this unsupported
> assertion. There's hardly any effective difference between a unique
> index + NOT NULL constraints and a declared primary key ... so what
> did you really need it for?

Agreed, functionally there's not much of a difference. It's more of a
matter of proper design identifying a primary key.

> > 1. Verify that the index named is a unique index
>
> ... and not partial, and not on expressions, and not invalid, and not
> using non-default opclasses (which might have a surprising definition of
> "equal"), and not already owned by a constraint ... not to mention that
> it'd better be an index on the named table, which among other things
> removes the need for a schema specification on the index name.

Of course.

--
Jonah H. Harris, Sr. Software Architect | phone: 732.331.1324
EnterpriseDB Corporation | fax: 732.331.1301
499 Thornall Street, 2nd Floor | jonah(dot)harris(at)enterprisedb(dot)com
Edison, NJ 08837 | http://www.enterprisedb.com/

On Tue, Apr 8, 2008 at 10:03 PM, Jonah H. Harris <jonah(dot)harris(at)gmail(dot)com> wrote:
> On Tue, Apr 8, 2008 at 9:04 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> > > I've run into a couple cases now where it would be helpful to easily
> > > assign an already-existing unique index as a primary key.
> >
> > You need to present a more convincing use-case than this unsupported
> > assertion. There's hardly any effective difference between a unique
> > index + NOT NULL constraints and a declared primary key ... so what
> > did you really need it for?
>
> Agreed, functionally there's not much of a difference. It's more of a
> matter of proper design identifying a primary key.
>

set right constraints it's good for documenting the system itself, i
like the idea...

--
regards,
Jaime Casanova

Tom Lane schrieb:
> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
>
>> I've run into a couple cases now where it would be helpful to easily
>> assign an already-existing unique index as a primary key.
>>
>
> You need to present a more convincing use-case than this unsupported
> assertion. There's hardly any effective difference between a unique
> index + NOT NULL constraints and a declared primary key ... so what
> did you really need it for?
>
>
In fact it seems to be necessary when connecting with ODBC, I had the
problem a month ago, MsSQL will not work correctly with connected tables
in a postgres database when there is no PK. NOT NULL and unique index
is not enough.

But I think it's overkill to add ALTER commands for this rare corner
case, maybe it's enough to set "indisprimary" on the index?

Added to TODO:

o Allow an existing index to be marked as a table's primary key

---------------------------------------------------------------------------

Jonah H. Harris wrote:
> Hey all,
>
> I've run into a couple cases now where it would be helpful to easily
> assign an already-existing unique index as a primary key. Unless I
> completely missed something, there's no way to do this now without a
> bit of catalog hackery.
>
> My implementation idea is as follows:
>
> Proposed Syntax (based on Oracle's syntax)
>
> ALTER TABLE foo ADD CONSTRAINT bar PRIMARY KEY USING INDEX schema.tablename;
>
> Proposed Implementation
>
> 1. Verify that the index named is a unique index
> 2. Check index columns for NOT NULL constraints
> 3. If indexed columns are not already NOT NULL, apply NOT NULL
> 4. If NOT NULL succeeds, complete the operation (catalogs,
> dependencies, ...), else bail out.
>
> Any comments, ideas, suggestions?
>
> --
> Jonah H. Harris, Sr. Software Architect | phone: 732.331.1324
> EnterpriseDB Corporation | fax: 732.331.1301
> 499 Thornall Street, 2nd Floor | jonah(dot)harris(at)enterprisedb(dot)com
> Edison, NJ 08837 | http://www.enterprisedb.com/
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Added to TODO:
>
>> Proposed Implementation
>>
>> 1. Verify that the index named is a unique index
>> 2. Check index columns for NOT NULL constraints
>> 3. If indexed columns are not already NOT NULL, apply NOT NULL
>> 4. If NOT NULL succeeds, complete the operation (catalogs,
>> dependencies, ...), else bail out.
>>
>> Any comments, ideas, suggestions?

I would add:

5. Modify index name to use appropriate naming style.

Joshua D. Drake

"Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> Bruce Momjian wrote:
>> Any comments, ideas, suggestions?

> I would add:

> 5. Modify index name to use appropriate naming style.

Why, and exactly what would you define as "appropriate naming style"?
The user has always been free to pick whatever constraint name he
wants.

regards, tom lane

Tom Lane wrote:
> "Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
>> Bruce Momjian wrote:
>>> Any comments, ideas, suggestions?
>
>> I would add:
>
>> 5. Modify index name to use appropriate naming style.
>
> Why, and exactly what would you define as "appropriate naming style"?
> The user has always been free to pick whatever constraint name he
> wants.

Well it should be optional but it would be nice if we had the option to
have it renamed per the default... meaning the same output if I were to
do this:

create table foo (id serial primary key);

I end up with "foo_pkey" PRIMARY KEY, btree (id)

Which is nice for consistency.

Sincerely,

Joshua D. Drake

>
> regards, tom lane
>

"Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> Tom Lane wrote:
>> Why, and exactly what would you define as "appropriate naming style"?
>> The user has always been free to pick whatever constraint name he
>> wants.

> Well it should be optional but it would be nice if we had the option to
> have it renamed per the default... meaning the same output if I were to
> do this:

If you want that, you can rename the index (either before or afterwards).
I don't see any reason to clutter the make-constraint-from-index command
with questions of renaming.

regards, tom lane

Tom Lane wrote:

>> Well it should be optional but it would be nice if we had the option to
>> have it renamed per the default... meaning the same output if I were to
>> do this:
>
> If you want that, you can rename the index (either before or afterwards).
> I don't see any reason to clutter the make-constraint-from-index command
> with questions of renaming.

As a counter point, I don't see any reason to make the DBA's life
harder. Sure it is just one step but it is a human step, prone to error
and taking more time than it should. Why not just make it easy?
Especially when the easy isn't sacrificing data integrity or quality of
product?

Sincerely,

Joshua D. Drake

>
> regards, tom lane
>

Joshua D. Drake wrote:
> Tom Lane wrote:
>
>>> Well it should be optional but it would be nice if we had the option
>>> to have it renamed per the default... meaning the same output if I
>>> were to do this:
>>
>> If you want that, you can rename the index (either before or
>> afterwards).
>> I don't see any reason to clutter the make-constraint-from-index command
>> with questions of renaming.
>
> As a counter point, I don't see any reason to make the DBA's life
> harder. Sure it is just one step but it is a human step, prone to
> error and taking more time than it should. Why not just make it easy?
> Especially when the easy isn't sacrificing data integrity or quality
> of product?
>
>
>

Because that's not the basis on which we decide to add features. You
need to asses the code complexity, the potential benefit and number of
likely users. In this case, the amount of code required for what would
be nothing more than syntactic sugar for what is in any case a very
simple statement makes me agree with Tom.

cheers

andrew

"Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:
> As a counter point, I don't see any reason to make the DBA's life
> harder. Sure it is just one step but it is a human step, prone to error
> and taking more time than it should. Why not just make it easy?

I don't see that decorating infrequently-used statements with bizarre
options that duplicate the functionality of other commands is "making it
easy". Apparently your definition of "easy" depends entirely on
keystrokes and not at all on memory/cognitive burden.

IMHO a utility command should do one easily-explained thing. The fewer
options the better.

regards, tom lane

Tom Lane wrote:

> Apparently your definition of "easy" depends entirely on
> keystrokes and not at all on memory/cognitive burden.

I was trying to remove one opportunity for human error, which is tied to
memory and cognitive burden. It is very easy to fat finger something. Is
it a critical error? No. Is it obnoxious to have to go back and fix it,
yes. When you are going back to fix, are you going to be grousing about
how PostgreSQL doesn't make this easier, maybe.

>
> IMHO a utility command should do one easily-explained thing. The fewer
> options the better.

I would agree with this except that by my definition your argument
fails. You are adding options by not allowing a sane default that
applies consistency to the database. I believe this will cause more
trouble than having the limitation in the first place.

Anyway, I have made my arguments. I believe we are still in the middle
of a commit fest.

Sincerely,

Joshua D. Drake

So, would anyone be averse to something like the following:

ALTER TABLE blah ADD ... PRIMARY KEY (...) USING PREBUILT INDEX index_hame

If the user doesn't specify CONSTRAINT constraint_name, it will
default to current implicit behavior of col_pkey.

-Jonah

On Sat, May 10, 2008 at 1:08 PM, Joshua D. Drake <jd(at)commandprompt(dot)com> wrote:
> Tom Lane wrote:
>
>> Apparently your definition of "easy" depends entirely on
>> keystrokes and not at all on memory/cognitive burden.
>
> I was trying to remove one opportunity for human error, which is tied to
> memory and cognitive burden. It is very easy to fat finger something. Is it
> a critical error? No. Is it obnoxious to have to go back and fix it, yes.
> When you are going back to fix, are you going to be grousing about how
> PostgreSQL doesn't make this easier, maybe.
>
>>
>> IMHO a utility command should do one easily-explained thing. The fewer
>> options the better.
>
> I would agree with this except that by my definition your argument fails.
> You are adding options by not allowing a sane default that applies
> consistency to the database. I believe this will cause more trouble than
> having the limitation in the first place.
>
> Anyway, I have made my arguments. I believe we are still in the middle of a
> commit fest.
>
> Sincerely,
>
> Joshua D. Drake
>
>

--
Jonah H. Harris, Sr. Software Architect | phone: 732.331.1324
EnterpriseDB Corporation | fax: 732.331.1301
499 Thornall Street, 2nd Floor | jonah(dot)harris(at)enterprisedb(dot)com
Edison, NJ 08837 | http://www.enterprisedb.com/

"Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:

> So, would anyone be averse to something like the following:
>
> ALTER TABLE blah ADD ... PRIMARY KEY (...) USING PREBUILT INDEX index_hame
>
> If the user doesn't specify CONSTRAINT constraint_name, it will
> default to current implicit behavior of col_pkey.

This is all so that the primary key shows up with a nice "PRIMARY KEY" instead
of just the unique index?

The "PREBUILT" seems unnecessary in that syntax.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's Slony Replication support!

Yes, I just think PREBUILT conveys the meaning of the command more
appropriately. I could care less though.

On Sat, May 10, 2008 at 5:35 PM, Gregory Stark <stark(at)enterprisedb(dot)com> wrote:
> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
>
>> So, would anyone be averse to something like the following:
>>
>> ALTER TABLE blah ADD ... PRIMARY KEY (...) USING PREBUILT INDEX index_hame
>>
>> If the user doesn't specify CONSTRAINT constraint_name, it will
>> default to current implicit behavior of col_pkey.
>
> This is all so that the primary key shows up with a nice "PRIMARY KEY" instead
> of just the unique index?
>
> The "PREBUILT" seems unnecessary in that syntax.
>
> --
> Gregory Stark
> EnterpriseDB http://www.enterprisedb.com
> Ask me about EnterpriseDB's Slony Replication support!
>

--
Jonah H. Harris, Sr. Software Architect | phone: 732.331.1324
EnterpriseDB Corporation | fax: 732.331.1301
499 Thornall Street, 2nd Floor | jonah(dot)harris(at)enterprisedb(dot)com
Edison, NJ 08837 | http://www.enterprisedb.com/

Jonah H. Harris wrote:
> Yes, I just think PREBUILT conveys the meaning of the command more
> appropriately. I could care less though.
>

(Please don't top-answer)

I don't think we should add new keywords unnecessarily.

cheers

andrew
> On Sat, May 10, 2008 at 5:35 PM, Gregory Stark <stark(at)enterprisedb(dot)com> wrote:
>
>> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
>>
>>
>>> So, would anyone be averse to something like the following:
>>>
>>> ALTER TABLE blah ADD ... PRIMARY KEY (...) USING PREBUILT INDEX index_hame
>>>
>>> If the user doesn't specify CONSTRAINT constraint_name, it will
>>> default to current implicit behavior of col_pkey.
>>>
>> This is all so that the primary key shows up with a nice "PRIMARY KEY" instead
>> of just the unique index?
>>
>> The "PREBUILT" seems unnecessary in that syntax.
>>
>> --
>> Gregory Stark
>> EnterpriseDB http://www.enterprisedb.com
>> Ask me about EnterpriseDB's Slony Replication support!
>>
>>
>
>
>
>

"Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> ALTER TABLE blah ADD ... PRIMARY KEY (...) USING PREBUILT INDEX index_hame

> If the user doesn't specify CONSTRAINT constraint_name, it will
> default to current implicit behavior of col_pkey.

IOW, the default behavior is to rename the index? Doesn't seem to me
to satisfy the principle of least surprise.

I agree with Andrew that creating new keywords just for noise purposes
is not gonna happen.

regards, tom lane

On Sat, May 10, 2008 at 11:55:29AM -0400, Tom Lane wrote:

> IMHO a utility command should do one easily-explained thing. The fewer
> options the better.

Sticking to that principle makes for a better-maintained system. I
agree. If we want to point out, "You might rename your index
afterwards to make it look like other default primary keys," I have no
objection.

A

--
Andrew Sullivan
ajs(at)commandprompt(dot)com
+1 503 667 4564 x104
http://www.commandprompt.com/

Joshua D. Drake wrote:
> Tom Lane wrote:
>
>>> Well it should be optional but it would be nice if we had the option
>>> to have it renamed per the default... meaning the same output if I
>>> were to do this:
>>
>> If you want that, you can rename the index (either before or afterwards).
>> I don't see any reason to clutter the make-constraint-from-index command
>> with questions of renaming.
>
> As a counter point, I don't see any reason to make the DBA's life
> harder. Sure it is just one step but it is a human step, prone to error
> and taking more time than it should. Why not just make it easy?
> Especially when the easy isn't sacrificing data integrity or quality of
> product?

well the name is by no means a functional problem. Its merely cosmetics,
so if you want propose that a warning is issued to suggest a saner name.

This should be sufficient I think.

T.

On Sat, May 10, 2008 at 10:41:56PM -0400, Andrew Sullivan wrote:
> On Sat, May 10, 2008 at 11:55:29AM -0400, Tom Lane wrote:
> > IMHO a utility command should do one easily-explained thing. The
> > fewer options the better.
>
> Sticking to that principle makes for a better-maintained system. I
> agree. If we want to point out, "You might rename your index
> afterwards to make it look like other default primary keys," I have
> no objection.

For convenience, it might be nice to include the generated name in the
notice.

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Joshua D. Drake wrote:
> Tom Lane wrote:
>
> >> Well it should be optional but it would be nice if we had the option to
> >> have it renamed per the default... meaning the same output if I were to
> >> do this:
> >
> > If you want that, you can rename the index (either before or afterwards).
> > I don't see any reason to clutter the make-constraint-from-index command
> > with questions of renaming.
>
> As a counter point, I don't see any reason to make the DBA's life
> harder. Sure it is just one step but it is a human step, prone to error
> and taking more time than it should. Why not just make it easy?
> Especially when the easy isn't sacrificing data integrity or quality of
> product?

I realize most feel we don't need to add a rename to this, but there are
two more reasons _not_ to do this. First, there is the possibility of
name collision with the new name so you would then require the user to
use the option not to rename. Plus, if you renamed, the old index name
would go away, and some people might think the index was removed and not
realize it was renamed, or find it confusing it was renamed.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> I realize most feel we don't need to add a rename to this, but there are
> two more reasons _not_ to do this.

One other thought I had about this is that the proposed syntax

ALTER TABLE tab ADD PRIMARY KEY (col [, ...]) USING INDEX foo

is not well chosen anyway. It forces the user to provide a column name
list matching the index, which is just extra typing and extra cognitive
burden, and it forces the system to have code checking that this list
matches the specified index. So I'm thinking it should look like

ALTER TABLE tab ADD PRIMARY KEY USING INDEX foo
or maybe just
ALTER TABLE tab ADD PRIMARY KEY USING foo

This would be a separate grammar production having nothing to do with
the ADD CONSTRAINT syntax. It's not ambiguous since the column name
list is required in ADD CONSTRAINT.

BTW, aside from selecting the index the command would have to verify
that the indexed columns are all NOT NULL. We could either have it
just throw an error if they aren't, or have it silently try to do
an ALTER SET NOT NULL, which would require a table scan.

I'm going to argue for the "just throw an error" choice. I don't like
the idea of a utility command that takes exclusive lock and then is
either near-instantaneous or slow depending on factors not immediately
obvious.

regards, tom lane

Tom Lane wrote:
> BTW, aside from selecting the index the command would have to verify
> that the indexed columns are all NOT NULL. We could either have it
> just throw an error if they aren't, or have it silently try to do
> an ALTER SET NOT NULL, which would require a table scan.
>
> I'm going to argue for the "just throw an error" choice. I don't like
> the idea of a utility command that takes exclusive lock and then is
> either near-instantaneous or slow depending on factors not immediately
> obvious.
>
>
>

+1

cheers

andrew