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

From: Steve Singer <ssinger_pg(at)sympatico(dot)ca>
To: Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>
Cc: PGSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, r t <pgsql(at)xzilla(dot)net>, Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>, Steve Singer <ssinger_pg(at)sympatico(dot)ca>
Subject: Re: Patch to add a primary key using an existing index
Date: 2011-01-16 22:34:14
Message-ID: BLU0-SMTP647ABAF0843BF54B9B9E9F8EF50@phx.gbl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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 Content-Type Size
existing_index_doc_patch.diff text/x-patch 2.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2011-01-16 22:37:16 Re: limiting hint bit I/O
Previous Message Noah Misch 2011-01-16 22:23:39 Re: texteq/byteaeq: avoid detoast [REVIEW]