-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

The attached patch resolves the bug reported by Kaloyan Iliev [1] on -general.

The problem occurred when executing ALTER TABLE ... ADD COLUMN ...
PRIMARY KEY with no default clause, on a table with rows already
present.  The NOT NULL constraint was not enforced.  The bug has been
observed on (at least) 8.2, 8.3 and CVS HEAD.

The fix was to force evaluation of the NOT NULL constraint in
ATExecAddColumn in all cases where the parser indicated that the
column ought to be NOT NULL.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: http://getfiregpg.org

iD8DBQFIENpl5YBsbHkuyV0RApP6AKDdnZAA0LjSzh9XdiQt7K/cx4YOcQCgydHr
6BYSD2kVX2DP7LYgDHvzNdE=
=Yrmp
-----END PGP SIGNATURE-----

On Fri, Apr 25, 2008 at 3:46 AM, Tom Lane <[EMAIL PROTECTED]> wrote:
>  alter table t1 add column f2 int not null;
>
>  transformAlterTableStmt will produce an AT_AddColumn subcommand
>  containing a ColumnDef with is_not_null = false, followed by an
>  AT_SetNotNull subcommand.  But for
>

I realised that there's no reason for preparing a separate SetNotNull
subcommand anymore, now that ATExecAddColumn takes care of enforcing
the constraint, so I removed this special case.

I've also added a regression test for ADD COLUMN PRIMARY KEY.

Cheers,
BJ

[1] http://archives.postgresql.org/message-id/[EMAIL PROTECTED]
*** src/backend/commands/tablecmds.c
--- src/backend/commands/tablecmds.c
***************
*** 3333,3338 **** ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
--- 3333,3345 ----
        }
  
        /*
+        * Tell Phase 3 to perform the check for NULL values in the new column 
if
+        * its attnotnull bit has been set to true.
+        */
+       if (attribute->attnotnull)
+               tab->new_notnull = true;
+ 
+       /*
         * Add needed dependency entries for the new column.
         */
        add_column_datatype_dependency(myrelid, i, attribute->atttypid);
*** src/backend/parser/parse_utilcmd.c
--- src/backend/parser/parse_utilcmd.c
***************
*** 1726,1753 **** transformAlterTableStmt(AlterTableStmt *stmt, const char 
*queryString)
                                         * If the column has a non-null 
default, we can't skip
                                         * validation of foreign keys.
                                         */
!                                       if (((ColumnDef *) 
cmd->def)->raw_default != NULL)
                                                skipValidation = false;
  
                                        newcmds = lappend(newcmds, cmd);
  
                                        /*
-                                        * Convert an ADD COLUMN ... NOT NULL 
constraint to a
-                                        * separate command
-                                        */
-                                       if (def->is_not_null)
-                                       {
-                                               /* Remove NOT NULL from 
AddColumn */
-                                               def->is_not_null = false;
- 
-                                               /* Add as a separate 
AlterTableCmd */
-                                               newcmd = 
makeNode(AlterTableCmd);
-                                               newcmd->subtype = AT_SetNotNull;
-                                               newcmd->name = 
pstrdup(def->colname);
-                                               newcmds = lappend(newcmds, 
newcmd);
-                                       }
- 
-                                       /*
                                         * All constraints are processed in 
other ways. Remove the
                                         * original list
                                         */
--- 1726,1737 ----
                                         * If the column has a non-null 
default, we can't skip
                                         * validation of foreign keys.
                                         */
!                                       if (def->raw_default != NULL)
                                                skipValidation = false;
  
                                        newcmds = lappend(newcmds, cmd);
  
                                        /*
                                         * All constraints are processed in 
other ways. Remove the
                                         * original list
                                         */
*** src/test/regress/expected/alter_table.out
--- src/test/regress/expected/alter_table.out
***************
*** 505,510 **** create table atacc1 ( test int );
--- 505,522 ----
  alter table atacc1 add constraint atacc_test1 primary key (test1);
  ERROR:  column "test1" named in key does not exist
  drop table atacc1;
+ -- Adding a new column as primary key to a non-empty table.  Should fail 
unless
+ -- the column has a default value.
+ create table atacc1 ( test int );
+ insert into atacc1 (test) values (0);
+ -- Add a primary key column without a default (fails).
+ alter table atacc1 add column test2 int primary key;
+ NOTICE:  ALTER TABLE / ADD PRIMARY KEY will create implicit index 
"atacc1_pkey" for table "atacc1"
+ ERROR:  column "test2" contains null values
+ -- Now add a primary key column with a default (succeeds).
+ alter table atacc1 add column test2 int default 0 primary key;
+ NOTICE:  ALTER TABLE / ADD PRIMARY KEY will create implicit index 
"atacc1_pkey" for table "atacc1"
+ drop table atacc1;
  -- something a little more complicated
  create table atacc1 ( test int, test2 int);
  -- add a primary key constraint
*** src/test/regress/sql/alter_table.sql
--- src/test/regress/sql/alter_table.sql
***************
*** 506,511 **** create table atacc1 ( test int );
--- 506,521 ----
  alter table atacc1 add constraint atacc_test1 primary key (test1);
  drop table atacc1;
  
+ -- Adding a new column as primary key to a non-empty table.  Should fail 
unless
+ -- the column has a default value.
+ create table atacc1 ( test int );
+ insert into atacc1 (test) values (0);
+ -- Add a primary key column without a default (fails).
+ alter table atacc1 add column test2 int primary key;
+ -- Now add a primary key column with a default (succeeds).
+ alter table atacc1 add column test2 int default 0 primary key;
+ drop table atacc1;
+ 
  -- something a little more complicated
  create table atacc1 ( test int, test2 int);
  -- add a primary key constraint
-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Reply via email to