Re: Patch to add a primary key using an existing index

<div dir="ltr"><div class="gmail_quote">On Sun, Dec 5, 2010 at 2:09 PM, Peter Eisentraut <span dir="ltr"><<a
href="mailto:peter_e@gmx.net">peter_e@gmx.net</a>></span>wrote:<br /><blockquote class="gmail_quote" style="margin:
0pt0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div class="im">On fre, 2010-12-03 at
15:27-0500, Robert Haas wrote:<br /> > On Fri, Dec 3, 2010 at 2:56 PM, r t <<a
href="mailto:pgsql@xzilla.net">pgsql@xzilla.net</a>>wrote:<br /> > > What exactly was the objection to the
following--><br /> > > ALTER TABLE table_name ADD PRIMARY KEY (column_list) USING index_name;<br /> > >
Isthe objection that you might have been trying to specify a constraint<br /> > > named "using" ? I'm willing to
makethat option more difficult. :-)<br /> ><br /> > I think it's that someone might expect the word after USING
tobe the<br /> > name of an index AM.<br /><br /></div>That could be avoided by writing<br /><br /> USING INDEX
<name><br/><br /></blockquote></div><br />Allowing USING INDEX along with USING INDEX TABLESPACE is causing
shift/reduceconflicts.<br /><br />I liked the proposal upthread of providing alternate syntax where user does not have
tospecify column-list and system picks up that list from the index.<br /><br clear="all" />ALTER TABLE table_name ADD
[CONSTRAINTcons_name] PRIMARY KEY (column_list) [WITH (...)] [USING INDEX TABLESPACE tblspcname];<br />ALTER TABLE
table_nameADD [CONSTRAINT cons_name] PRIMARY KEY [WITH (...)] [USING INDEX index_name];<br /><br />This would also help
avoidthe bug that Itagaki found, where the user wants to use an existing index, and also specifies USING INDEX
TABLESPACE.<br/><br />But I still hold a bias towards renaming the index to match constraint name (with a NOTICE),
ratherthan require that the constraint name match the index name, because the constraint name is optional and when it
isnot provided system has to generate a name and we have to rename the index anyway to maintain consistency.<br /><br
/>Followingare the gram.y changes that I am going to start with:<br /><br /> %type <boolean>
constraints_set_mode<br/>-%type <str>        OptTableSpace OptConsTableSpace OptTableSpaceOwner<br />+%type
<str>       OptTableSpace OptConsTableSpace OptConsIndex OptTableSpaceOwner<br />  %type <list>  
opt_check_option<br/><br />[...]<br />            | UNIQUE '(' columnList ')' opt_definition OptConsTableSpace<br
/>               ConstraintAttributeSpec<br />                {<br />                    Constraint *n =
makeNode(Constraint);<br/>                     n->contype = CONSTR_UNIQUE;<br />                    n->location =
@1;<br/>                    n->keys = $3;<br />                    n->options = $5;<br />                   
n->indexspace= $6;<br />                     n->deferrable = ($7 & 1) != 0;<br />                   
n->initdeferred= ($7 & 2) != 0;<br />                    $$ = (Node *)n;<br />                }<br />+          
|UNIQUE opt_definition OptConsIndex ConstraintAttributeSpec<br /> +               {<br />+                   Constraint
*n= makeNode(Constraint);<br />+                   n->contype = CONSTR_UNIQUE;<br />+                  
n->location= @1;<br />+                   n->options = $2;<br /> +                   n->indexname = $3;<br
/>+                  n->deferrable = ($4 & 1) != 0;<br />+                   n->initdeferred = ($4 & 2)
!=0;<br />+                   $$ = (Node *)n;<br />+               }<br />             | PRIMARY KEY '(' columnList ')'
opt_definitionOptConsTableSpace<br />                ConstraintAttributeSpec<br />                {<br
/>                   Constraint *n = makeNode(Constraint);<br />                     n->contype = CONSTR_PRIMARY;<br
/>                   n->location = @1;<br />                    n->keys = $4;<br />                   
n->options= $6;<br />                    n->indexspace = $7;<br />                     n->deferrable = ($8
&1) != 0;<br />                    n->initdeferred = ($8 & 2) != 0;<br />                    $$ = (Node
*)n;<br/>                }<br />+           | PRIMARY KEY opt_definition OptConsIndex ConstraintAttributeSpec<br />
+              {<br />+                   Constraint *n = makeNode(Constraint);<br />+                   n->contype
=CONSTR_PRIMARY;<br />+                   n->location = @1;<br />+                   n->options = $3;<br />
+                  n->indexname = $4;<br />+                   n->deferrable = ($5 & 1) != 0;<br
/>+                  n->initdeferred = ($5 & 2) != 0;<br />+                   $$ = (Node *)n;<br
/>+              }<br />             | EXCLUDE access_method_clause '(' ExclusionConstraintList ')'<br /><br />[...]<br
/> OptConsTableSpace:  USING INDEX TABLESPACE name   { $$ = $4; }<br />            |
/*EMPTY*/                            { $$ = NULL; }<br />         ;<br /> <br />+OptConsIndex:   USING INDEX name   {
$$= $3; }<br />+           | /*EMPTY*/             { $$ = NULL; }<br />+       ;<br />+<br /><br />Regards,<br />-- <br
/>gurjeet.singh<br/>@ EnterpriseDB - The Enterprise Postgres Company<br /><a
href="http://www.EnterpriseDB.com">http://www.EnterpriseDB.com</a><br/><br />singh.gurjeet@{ gmail | yahoo }.com<br
/>Twitter/Skype:singh_gurjeet<br /><br />Mail sent from my BlackLaptop device<br /></div>