On Thu, Apr 23, 2015 at 12:05 PM, Fabrízio de Royes Mello <
fabriziome...@gmail.com> wrote:
>
>
> On Wed, Apr 22, 2015 at 3:48 PM, Payal Singh <pa...@omniti.com> wrote:
> >
> > The following review has been posted through the commitfest application:
> > make installcheck-world: tested, failed
> > Implements feature: not tested
> > Spec compliant: not tested
> > Documentation: not tested
> >
> > Seeing this when trying to apply the patch:
> >
> > Patching file src/backend/commands/tablecmds.c using Plan A...
> > Hunk #1 FAILED at 328.
> > Hunk #2 succeeded at 2294 (offset 11 lines).
> > Hunk #3 FAILED at 3399.
> > Hunk #4 FAILED at 3500.
> > Hunk #5 succeeded at 4658 with fuzz 1 (offset 65 lines).
> > Hunk #6 succeeded at 4753 (offset 66 lines).
> > Hunk #7 succeeded at 4989 with fuzz 2 (offset 66 lines).
> > Hunk #8 succeeded at 5003 (offset 69 lines).
> > Hunk #9 succeeded at 5017 (offset 69 lines).
> > Hunk #10 succeeded at 5033 (offset 69 lines).
> >
> > The new status of this patch is: Waiting on Author
> >
>
> The patch needs a "rebase". Done!
>
Another rebased version.
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 207fec1..339320e 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -36,7 +36,7 @@ ALTER TABLE ALL IN TABLESPACE <replaceable class="PARAMETER">name</replaceable>
<phrase>where <replaceable class="PARAMETER">action</replaceable> is one of:</phrase>
- ADD [ COLUMN ] <replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ COLLATE <replaceable class="PARAMETER">collation</replaceable> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
+ ADD [ COLUMN ] [ IF NOT EXISTS ]<replaceable class="PARAMETER">column_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ COLLATE <replaceable class="PARAMETER">collation</replaceable> ] [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
DROP [ COLUMN ] [ IF EXISTS ] <replaceable class="PARAMETER">column_name</replaceable> [ RESTRICT | CASCADE ]
ALTER [ COLUMN ] <replaceable class="PARAMETER">column_name</replaceable> [ SET DATA ] TYPE <replaceable class="PARAMETER">data_type</replaceable> [ COLLATE <replaceable class="PARAMETER">collation</replaceable> ] [ USING <replaceable class="PARAMETER">expression</replaceable> ]
ALTER [ COLUMN ] <replaceable class="PARAMETER">column_name</replaceable> SET DEFAULT <replaceable class="PARAMETER">expression</replaceable>
@@ -96,11 +96,12 @@ ALTER TABLE ALL IN TABLESPACE <replaceable class="PARAMETER">name</replaceable>
<variablelist>
<varlistentry>
- <term><literal>ADD COLUMN</literal></term>
+ <term><literal>ADD COLUMN [ IF NOT EXISTS ]</literal></term>
<listitem>
<para>
This form adds a new column to the table, using the same syntax as
- <xref linkend="SQL-CREATETABLE">.
+ <xref linkend="SQL-CREATETABLE">. If <literal>IF NOT EXISTS</literal>
+ is specified and the column already exists, no error is thrown.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d394713..2257ca2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -306,14 +306,14 @@ static void createForeignKeyTriggers(Relation rel, Oid refRelOid,
Constraint *fkconstraint,
Oid constraintOid, Oid indexOid);
static void ATController(AlterTableStmt *parsetree,
- Relation rel, List *cmds, bool recurse, LOCKMODE lockmode);
+ Relation rel, List *cmds, bool recurse, LOCKMODE lockmode);
static void ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
bool recurse, bool recursing, LOCKMODE lockmode);
static void ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode);
static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
AlterTableCmd *cmd, LOCKMODE lockmode);
static void ATRewriteTables(AlterTableStmt *parsetree,
- List **wqueue, LOCKMODE lockmode);
+ List **wqueue, LOCKMODE lockmode);
static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode);
static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel);
static void ATSimplePermissions(Relation rel, int allowed_targets);
@@ -328,8 +328,8 @@ static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recu
bool is_view, AlterTableCmd *cmd, LOCKMODE lockmode);
static ObjectAddress ATExecAddColumn(List **wqueue, AlteredTableInfo *tab,
Relation rel, ColumnDef *colDef, bool isOid,
- bool recurse, bool recursing, LOCKMODE lockmode);
-static void check_for_column_name_collision(Relation rel, const char *colname);
+ bool recurse, bool recursing, bool if_not_exists, LOCKMODE lockmode);
+static bool check_for_column_name_collision(Relation rel, const char *colname, bool if_not_exists);
static void add_column_datatype_dependency(Oid relid, int32 attnum, Oid typid);
static void add_column_collation_dependency(Oid relid, int32 attnum, Oid collid);
static void ATPrepAddOids(List **wqueue, Relation rel, bool recurse,
@@ -631,7 +631,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
cooked = (CookedConstraint *) palloc(sizeof(CookedConstraint));
cooked->contype = CONSTR_DEFAULT;
- cooked->conoid = InvalidOid; /* until created */
+ cooked->conoid = InvalidOid; /* until created */
cooked->name = NULL;
cooked->attnum = attnum;
cooked->expr = colDef->cooked_default;
@@ -1751,7 +1751,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
cooked = (CookedConstraint *) palloc(sizeof(CookedConstraint));
cooked->contype = CONSTR_CHECK;
- cooked->conoid = InvalidOid; /* until created */
+ cooked->conoid = InvalidOid; /* until created */
cooked->name = pstrdup(name);
cooked->attnum = 0; /* not used for constraints */
cooked->expr = expr;
@@ -1781,7 +1781,7 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
*/
if (inhSchema != NIL)
{
- int schema_attno = 0;
+ int schema_attno = 0;
foreach(entry, schema)
{
@@ -1809,14 +1809,14 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
* Yes, try to merge the two column definitions. They must
* have the same type, typmod, and collation.
*/
- if (exist_attno == schema_attno)
+ if (exist_attno == schema_attno)
ereport(NOTICE,
- (errmsg("merging column \"%s\" with inherited definition",
- attributeName)));
+ (errmsg("merging column \"%s\" with inherited definition",
+ attributeName)));
else
ereport(NOTICE,
- (errmsg("moving and merging column \"%s\" with inherited definition", attributeName),
- errdetail("User-specified column moved to the position of the inherited column.")));
+ (errmsg("moving and merging column \"%s\" with inherited definition", attributeName),
+ errdetail("User-specified column moved to the position of the inherited column.")));
def = (ColumnDef *) list_nth(inhSchema, exist_attno - 1);
typenameTypeIdAndMod(NULL, def->typeName, &defTypeId, &deftypmod);
typenameTypeIdAndMod(NULL, newdef->typeName, &newTypeId, &newtypmod);
@@ -2302,7 +2302,7 @@ renameatt_internal(Oid myrelid,
oldattname)));
/* new name should not already exist */
- check_for_column_name_collision(targetrelation, newattname);
+ check_for_column_name_collision(targetrelation, newattname, false);
/* apply the update */
namestrcpy(&(attform->attname), newattname);
@@ -3453,11 +3453,11 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
case AT_AddColumnToView: /* add column via CREATE OR REPLACE
* VIEW */
address = ATExecAddColumn(wqueue, tab, rel, (ColumnDef *) cmd->def,
- false, false, false, lockmode);
+ false, false, false, false, lockmode);
break;
case AT_AddColumnRecurse:
address = ATExecAddColumn(wqueue, tab, rel, (ColumnDef *) cmd->def,
- false, true, false, lockmode);
+ false, true, false, cmd->missing_ok, lockmode);
break;
case AT_ColumnDefault: /* ALTER COLUMN DEFAULT */
address = ATExecColumnDefault(rel, cmd->name, cmd->def, lockmode);
@@ -3496,7 +3496,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
break;
case AT_ReAddIndex: /* ADD INDEX */
address = ATExecAddIndex(tab, rel, (IndexStmt *) cmd->def, true,
- lockmode);
+ lockmode);
break;
case AT_AddConstraint: /* ADD CONSTRAINT */
address =
@@ -3567,14 +3567,14 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
if (cmd->def != NULL)
address =
ATExecAddColumn(wqueue, tab, rel, (ColumnDef *) cmd->def,
- true, false, false, lockmode);
+ true, false, false, cmd->missing_ok, lockmode);
break;
case AT_AddOidsRecurse: /* SET WITH OIDS */
/* Use the ADD COLUMN code, unless prep decided to do nothing */
if (cmd->def != NULL)
address =
ATExecAddColumn(wqueue, tab, rel, (ColumnDef *) cmd->def,
- true, true, false, lockmode);
+ true, true, false, cmd->missing_ok, lockmode);
break;
case AT_DropOids: /* SET WITHOUT OIDS */
@@ -3803,7 +3803,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode)
* And fire it only once.
*/
if (parsetree)
- EventTriggerTableRewrite((Node *) parsetree,
+ EventTriggerTableRewrite((Node *)parsetree,
tab->relid,
tab->rewrite);
@@ -4672,7 +4672,7 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
static ObjectAddress
ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
ColumnDef *colDef, bool isOid,
- bool recurse, bool recursing, LOCKMODE lockmode)
+ bool recurse, bool recursing, bool if_not_exists, LOCKMODE lockmode)
{
Oid myrelid = RelationGetRelid(rel);
Relation pgclass,
@@ -4767,7 +4767,13 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
relkind = ((Form_pg_class) GETSTRUCT(reltup))->relkind;
/* new name should not already exist */
- check_for_column_name_collision(rel, colDef->colname);
+ if (!check_for_column_name_collision(rel, colDef->colname, if_not_exists))
+ {
+ heap_close(attrdesc, RowExclusiveLock);
+ heap_freetuple(reltup);
+ heap_close(pgclass, RowExclusiveLock);
+ return InvalidObjectAddress;
+ }
/* Determine the new attribute's number */
if (isOid)
@@ -4997,7 +5003,8 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
/* Recurse to child; return value is ignored */
ATExecAddColumn(wqueue, childtab, childrel,
- colDef, isOid, recurse, true, lockmode);
+ colDef, isOid, recurse, true,
+ if_not_exists, lockmode);
heap_close(childrel, NoLock);
}
@@ -5010,8 +5017,8 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
* If a new or renamed column will collide with the name of an existing
* column, error out.
*/
-static void
-check_for_column_name_collision(Relation rel, const char *colname)
+static bool
+check_for_column_name_collision(Relation rel, const char *colname, bool if_not_exists)
{
HeapTuple attTuple;
int attnum;
@@ -5024,7 +5031,7 @@ check_for_column_name_collision(Relation rel, const char *colname)
ObjectIdGetDatum(RelationGetRelid(rel)),
PointerGetDatum(colname));
if (!HeapTupleIsValid(attTuple))
- return;
+ return true;
attnum = ((Form_pg_attribute) GETSTRUCT(attTuple))->attnum;
ReleaseSysCache(attTuple);
@@ -5040,10 +5047,23 @@ check_for_column_name_collision(Relation rel, const char *colname)
errmsg("column name \"%s\" conflicts with a system column name",
colname)));
else
+ {
+ if (if_not_exists)
+ {
+ ereport(NOTICE,
+ (errcode(ERRCODE_DUPLICATE_COLUMN),
+ errmsg("column \"%s\" of relation \"%s\" already exists, skipping",
+ colname, RelationGetRelationName(rel))));
+ return false;
+ }
+
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_COLUMN),
errmsg("column \"%s\" of relation \"%s\" already exists",
colname, RelationGetRelationName(rel))));
+ }
+
+ return true;
}
/*
@@ -5960,7 +5980,7 @@ ATExecAddIndexConstraint(AlteredTableInfo *tab, Relation rel,
true, /* update pg_index */
true, /* remove old dependencies */
allowSystemTableMods,
- false); /* is_internal */
+ false); /* is_internal */
index_close(indexRel, NoLock);
@@ -6906,7 +6926,7 @@ ATExecValidateConstraint(Relation rel, char *constrName, bool recurse,
HeapTupleGetOid(tuple));
}
else
- address = InvalidObjectAddress; /* already validated */
+ address = InvalidObjectAddress; /* already validated */
systable_endscan(scan);
@@ -7900,8 +7920,8 @@ ATPrepAlterColumnType(List **wqueue,
else
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("column \"%s\" cannot be cast automatically to type %s",
- colName, format_type_be(targettype)),
+ errmsg("column \"%s\" cannot be cast automatically to type %s",
+ colName, format_type_be(targettype)),
/* translator: USING is SQL, don't translate it */
errhint("You might need to specify \"USING %s::%s\".",
quote_identifier(colName),
@@ -9717,9 +9737,9 @@ AlterTableMoveAll(AlterTableMoveAllStmt *stmt)
!ConditionalLockRelationOid(relOid, AccessExclusiveLock))
ereport(ERROR,
(errcode(ERRCODE_OBJECT_IN_USE),
- errmsg("aborting because lock on relation \"%s\".\"%s\" is not available",
- get_namespace_name(relForm->relnamespace),
- NameStr(relForm->relname))));
+ errmsg("aborting because lock on relation \"%s\".\"%s\" is not available",
+ get_namespace_name(relForm->relnamespace),
+ NameStr(relForm->relname))));
else
LockRelationOid(relOid, AccessExclusiveLock);
@@ -10939,9 +10959,9 @@ ATExecReplicaIdentity(Relation rel, ReplicaIdentityStmt *stmt, LOCKMODE lockmode
static void
ATExecEnableRowSecurity(Relation rel)
{
- Relation pg_class;
- Oid relid;
- HeapTuple tuple;
+ Relation pg_class;
+ Oid relid;
+ HeapTuple tuple;
relid = RelationGetRelid(rel);
@@ -10965,9 +10985,9 @@ ATExecEnableRowSecurity(Relation rel)
static void
ATExecDisableRowSecurity(Relation rel)
{
- Relation pg_class;
- Oid relid;
- HeapTuple tuple;
+ Relation pg_class;
+ Oid relid;
+ HeapTuple tuple;
relid = RelationGetRelid(rel);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index e0ff6f1..a7b09e4 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -1941,6 +1941,16 @@ alter_table_cmd:
AlterTableCmd *n = makeNode(AlterTableCmd);
n->subtype = AT_AddColumn;
n->def = $2;
+ n->missing_ok = false;
+ $$ = (Node *)n;
+ }
+ /* ALTER TABLE <name> ADD IF NOT EXISTS <coldef> */
+ | ADD_P IF_P NOT EXISTS columnDef
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_AddColumn;
+ n->def = $5;
+ n->missing_ok = true;
$$ = (Node *)n;
}
/* ALTER TABLE <name> ADD COLUMN <coldef> */
@@ -1949,6 +1959,16 @@ alter_table_cmd:
AlterTableCmd *n = makeNode(AlterTableCmd);
n->subtype = AT_AddColumn;
n->def = $3;
+ n->missing_ok = false;
+ $$ = (Node *)n;
+ }
+ /* ALTER TABLE <name> ADD COLUMN IF NOT EXISTS <coldef> */
+ | ADD_P COLUMN IF_P NOT EXISTS columnDef
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_AddColumn;
+ n->def = $6;
+ n->missing_ok = true;
$$ = (Node *)n;
}
/* ALTER TABLE <name> ALTER [COLUMN] <colname> {SET DEFAULT <expr>|DROP DEFAULT} */
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 3ad2c55..ae81ff6 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2542,3 +2542,92 @@ ALTER TABLE logged1 SET UNLOGGED; -- silently do nothing
DROP TABLE logged3;
DROP TABLE logged2;
DROP TABLE logged1;
+-- test ADD COLUMN IF NOT EXISTS
+CREATE TABLE test_add_column(c1 integer);
+\d test_add_column
+Table "public.test_add_column"
+ Column | Type | Modifiers
+--------+---------+-----------
+ c1 | integer |
+
+ALTER TABLE test_add_column
+ ADD COLUMN c2 integer;
+\d test_add_column
+Table "public.test_add_column"
+ Column | Type | Modifiers
+--------+---------+-----------
+ c1 | integer |
+ c2 | integer |
+
+ALTER TABLE test_add_column
+ ADD COLUMN c2 integer; -- fail because c2 already exists
+ERROR: column "c2" of relation "test_add_column" already exists
+\d test_add_column
+Table "public.test_add_column"
+ Column | Type | Modifiers
+--------+---------+-----------
+ c1 | integer |
+ c2 | integer |
+
+ALTER TABLE test_add_column
+ ADD COLUMN IF NOT EXISTS c2 integer; -- skipping because c2 already exists
+NOTICE: column "c2" of relation "test_add_column" already exists, skipping
+\d test_add_column
+Table "public.test_add_column"
+ Column | Type | Modifiers
+--------+---------+-----------
+ c1 | integer |
+ c2 | integer |
+
+ALTER TABLE test_add_column
+ ADD COLUMN c2 integer, -- fail because c2 already exists
+ ADD COLUMN c3 integer;
+ERROR: column "c2" of relation "test_add_column" already exists
+\d test_add_column
+Table "public.test_add_column"
+ Column | Type | Modifiers
+--------+---------+-----------
+ c1 | integer |
+ c2 | integer |
+
+ALTER TABLE test_add_column
+ ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exists
+ ADD COLUMN c3 integer; -- fail because c3 already exists
+NOTICE: column "c2" of relation "test_add_column" already exists, skipping
+\d test_add_column
+Table "public.test_add_column"
+ Column | Type | Modifiers
+--------+---------+-----------
+ c1 | integer |
+ c2 | integer |
+ c3 | integer |
+
+ALTER TABLE test_add_column
+ ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exists
+ ADD COLUMN IF NOT EXISTS c3 integer; -- skipping because c3 already exists
+NOTICE: column "c2" of relation "test_add_column" already exists, skipping
+NOTICE: column "c3" of relation "test_add_column" already exists, skipping
+\d test_add_column
+Table "public.test_add_column"
+ Column | Type | Modifiers
+--------+---------+-----------
+ c1 | integer |
+ c2 | integer |
+ c3 | integer |
+
+ALTER TABLE test_add_column
+ ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exists
+ ADD COLUMN IF NOT EXISTS c3 integer, -- skipping because c3 already exists
+ ADD COLUMN c4 integer;
+NOTICE: column "c2" of relation "test_add_column" already exists, skipping
+NOTICE: column "c3" of relation "test_add_column" already exists, skipping
+\d test_add_column
+Table "public.test_add_column"
+ Column | Type | Modifiers
+--------+---------+-----------
+ c1 | integer |
+ c2 | integer |
+ c3 | integer |
+ c4 | integer |
+
+DROP TABLE test_add_column;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 29c1875..bf7654c 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1688,3 +1688,34 @@ ALTER TABLE logged1 SET UNLOGGED; -- silently do nothing
DROP TABLE logged3;
DROP TABLE logged2;
DROP TABLE logged1;
+
+-- test ADD COLUMN IF NOT EXISTS
+CREATE TABLE test_add_column(c1 integer);
+\d test_add_column
+ALTER TABLE test_add_column
+ ADD COLUMN c2 integer;
+\d test_add_column
+ALTER TABLE test_add_column
+ ADD COLUMN c2 integer; -- fail because c2 already exists
+\d test_add_column
+ALTER TABLE test_add_column
+ ADD COLUMN IF NOT EXISTS c2 integer; -- skipping because c2 already exists
+\d test_add_column
+ALTER TABLE test_add_column
+ ADD COLUMN c2 integer, -- fail because c2 already exists
+ ADD COLUMN c3 integer;
+\d test_add_column
+ALTER TABLE test_add_column
+ ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exists
+ ADD COLUMN c3 integer; -- fail because c3 already exists
+\d test_add_column
+ALTER TABLE test_add_column
+ ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exists
+ ADD COLUMN IF NOT EXISTS c3 integer; -- skipping because c3 already exists
+\d test_add_column
+ALTER TABLE test_add_column
+ ADD COLUMN IF NOT EXISTS c2 integer, -- skipping because c2 already exists
+ ADD COLUMN IF NOT EXISTS c3 integer, -- skipping because c3 already exists
+ ADD COLUMN c4 integer;
+\d test_add_column
+DROP TABLE test_add_column;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
