Re: Patch to add a primary key using an existing index - Mailing list pgsql-hackers

From Steve Singer
Subject Re: Patch to add a primary key using an existing index
Date
Msg-id BLU0-SMTP647ABAF0843BF54B9B9E9F8EF50@phx.gbl
Whole thread Raw
In response to Re: Patch to add a primary key using an existing index  (Gurjeet Singh <singh.gurjeet@gmail.com>)
Responses Re: Patch to add a primary key using an existing index  (Robert Haas <robertmhaas@gmail.com>)
Re: Patch to add a primary key using an existing index  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
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



Attachment

pgsql-hackers by date:

Previous
From: Noah Misch
Date:
Subject: Re: texteq/byteaeq: avoid detoast [REVIEW]
Next
From: "Kevin Grittner"
Date:
Subject: Re: limiting hint bit I/O