On 10-11-22 09:37 AM, Gurjeet Singh wrote:
On Sat, Nov 20, 2010 at 9:00 AM, Steve Singer <ssinger...@sympatico.ca <mailto:ssinger...@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.gurj...@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet

Mail sent from my BlackLaptop device




Reply via email to