On 10-11-22 09:37 AM, Gurjeet Singh wrote:
On Sat, Nov 20, 2010 at 9:00 AM, Steve Singer
<ssinger_pg@sympatico.ca> wrote:
Submission Review:
========================
Tests
--------
The expected output for the regression tests you added don't match
what I'm getting when I run the tests with your patch applied.
I think you just need to regenerate the expected results they seem
to be from a previous version of the patch (different error messages etc..).
Fixed. Also modified one test to cover the case where constraint name is provided.
Almost fixed.
I still get an unexpected difference.
! DETAIL: cannot create PRIMARY KEY/UNIQUE constraint with a non-unique index.
CREATE UNIQUE INDEX rpi_idx2 ON rpi_test(a , b);
-- should fail; WITH INDEX option specified more than once.
ALTER TABLE rpi_test ADD PRIMARY KEY (a, b)
--- 35,41 ----
-- should fail; non-unique
ALTER TABLE rpi_test ADD primary key(a, b) WITH (INDEX = 'rpi_idx1');
ERROR: "rpi_idx1" is not a unique index
! DETAIL: Cannot create PRIMARY KEY/UNIQUE constraint using a non-unique index.
Documentation
---------------
I was able to generate the docs.
The ALTER TABLE page under the synopsis has
ADD table_constraint
where table_constraint is defined on the CREATE TABLE page.
On the CREATE TABLE page table_constraint isn't defined as having the WITH
, the WITH is part of index_parameters.
I propose the alter table page instead have
ADD table_constraint [index_parameters]
where index_parameters also references the CREATE TABLE page like table_constraint.
IMHO index_parameters is an optional component of table_constraint, and hence can't be mentioned here, at least not the way shown above.
My reading of CREATE TABLE is that index_parameters is an optional parameter that comes after table_constraint and isn't part of table_constraint. Any other opinions?
Everything else I mentioned seems fixed in this version
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.EnterpriseDB.com singh.gurjeet@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet
Mail sent from my BlackLaptop device