I've taken a look at this version of the patch.
Submission Review
----------------
This version of the patch applies cleanly to master. It matches your git
repo and includes test + docs.
Usability Review
---------------
The command syntax now matches what was discussed during the last cf.
The text of the notice:
test=# alter table a add constraint acons unique using index aind2;
NOTICE: ALTER TABLE / ADD UNIQUE USING INDEX will rename index "aind2"
to "acons"
Documentation
----------
I've attached a patch (to be applied on top of your latest patch) with
some editorial changes I'd recommend to your documentation. I feel it
reads a bit clearer (but others should chime in if they disagree or have
better wordings)
A git tree with changes rebased to master + this change is available
at https://github.com/ssinger/postgres/tree/ssinger/constraint_with_index
Code Review
-----------
src/backend/parser/parse_utilcmd.c: 1452
Your calling strdup on the attribute name. I don't have a good enough
grasp on the code to be able to trace this through to where the memory
gets free'd. Does it get freed? Should/could this be a call to pstrdup
Feature Test
-------------
I wasn't able to find any issues in my testing
I'm marking this as returned with feedback pending your answer on the
possible memory leak above but I think the patch is very close to being
ready.
Steve Singer
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 83d2fbb..0b486ab 100644
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
*************** ALTER TABLE <replaceable class="PARAMETE
*** 242,268 ****
<term><literal>ADD <replaceable class="PARAMETER">table_constraint_using_index</replaceable></literal></term>
<listitem>
<para>
! This form adds a new <literal>PRIMARY KEY</> or <literal>UNIQUE</>
constraint to the table using an existing index. All the columns of the
index will be included in the constraint.
</para>
<para>
! The index should be UNIQUE, and should not be a <firstterm>partial index</>
! or an <firstterm>expressional index</>.
</para>
<para>
! This can be helpful in situations where one wishes to recreate or
! <literal>REINDEX</> the index of a <literal>PRIMARY KEY</> or a
! <literal>UNIQUE</> constraint, but dropping and recreating the constraint
! to acheive the effect is not desirable. See the illustrative example below.
</para>
<note>
<para>
If a constraint name is provided then the index will be renamed to that
! name, else the constraint will be named to match the index name.
</para>
</note>
--- 242,270 ----
<term><literal>ADD <replaceable class="PARAMETER">table_constraint_using_index</replaceable></literal></term>
<listitem>
<para>
! This form adds a new <literal>PRIMARY KEY</> or <literal>UNIQUE</literal>
constraint to the table using an existing index. All the columns of the
index will be included in the constraint.
</para>
<para>
! The index should be a UNIQUE index. A <firstterm>partial index</firstterm>
! or an <firstterm>expressional index</firstterm> is not allowed.
</para>
<para>
! Adding a constraint using an existing index can be helpful in situations
! where you wishes to rebuild an index used for a
! <literal>PRIMARY KEY</literal> or a <literal>UNIQUE</literal> constraint,
! but dropping and recreating the constraint
! is not desirable. See the illustrative example below.
</para>
<note>
<para>
If a constraint name is provided then the index will be renamed to that
! name of the constraint. Otherwise the constraint will be named to match
! the index name.
</para>
</note>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
