Re: [HACKERS] Add CINE for ALTER TABLE ... ADD COLUMN
On Thu, Jul 23, 2015 at 9:55 AM, Fabrízio de Royes Mello wrote:
> Thank you for the review.
+ /* skipp if the name already exists and if_not_exists is true */
s/skipp/skip.
Except that this looks in good shape to me (see attached for a version
fixing the typo) so switched to "Ready for committer".
--
Michael
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 207fec1..5a71c3c 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 name
where action is one of:
-ADD [ COLUMN ] column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ]
+ADD [ COLUMN ] [ IF NOT EXISTS ] column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ]
DROP [ COLUMN ] [ IF EXISTS ] column_name [ RESTRICT | CASCADE ]
ALTER [ COLUMN ] column_name [ SET DATA ] TYPE data_type [ COLLATE collation ] [ USING expression ]
ALTER [ COLUMN ] column_name SET DEFAULT expression
@@ -96,11 +96,12 @@ ALTER TABLE ALL IN TABLESPACE name
-ADD COLUMN
+ADD COLUMN [ IF NOT EXISTS ]
This form adds a new column to the table, using the same syntax as
- .
+ . If IF NOT EXISTS
+ is specified and the column already exists, no error is thrown.
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1c7eded..92b2662 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -328,8 +328,9 @@ 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,
@@ -2304,7 +2305,7 @@ renameatt_internal(Oid myrelid,
oldattname)));
/* new name should not already exist */
- check_for_column_name_collision(targetrelation, newattname);
+ (void) check_for_column_name_collision(targetrelation, newattname, false);
/* apply the update */
namestrcpy(&(attform->attname), newattname);
@@ -3455,11 +3456,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);
@@ -3572,14 +3573,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 */
@@ -4677,7 +4678,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,
@@ -4771,8 +4772,14 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
elog(ERROR, "cache lookup failed for relation %u", myrelid);
relkind = ((Form_pg_class) GETSTRUCT(reltup))->relkind;
- /* new name should not already exist */
- check_for_column_name_collision(rel, colDef->colname);
+ /* skip if the name already exists and if_not_exists is true */
+ if (!check_for_column_name_collision(rel, colDef->colname, if_not_exists))
+ {
+ heap_close(attrdesc, RowExclusive
Re: [HACKERS] Add CINE for ALTER TABLE ... ADD COLUMN
On Thu, Jul 16, 2015 at 10:36 PM, Michael Paquier wrote: > > I had a look at this patch, and here are some minor comments: > 1) In alter_table.sgml, you need a space here: > [ IF NOT EXISTS ] 2) > + check_for_column_name_collision(targetrelation, newattname, false); > (void) needs to be added in front of check_for_column_name_collision > where its return value is not checked or static code analyzers are > surely going to complain. Fixed. > 3) Something minor, some lines of codes exceed 80 characters, see > declaration of check_for_column_name_collision for example... Fixed. > 4) This comment needs more precisions? > /* 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)) > The new name can actually exist if if_not_exists is true. > Improved the comment. > Except that the implementation looks sane to me. > Thank you for the review. Att, -- 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..5a71c3c 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 name where action is one of: -ADD [ COLUMN ] column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ] +ADD [ COLUMN ] [ IF NOT EXISTS ] column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ] DROP [ COLUMN ] [ IF EXISTS ] column_name [ RESTRICT | CASCADE ] ALTER [ COLUMN ] column_name [ SET DATA ] TYPE data_type [ COLLATE collation ] [ USING expression ] ALTER [ COLUMN ] column_name SET DEFAULT expression @@ -96,11 +96,12 @@ ALTER TABLE ALL IN TABLESPACE name -ADD COLUMN +ADD COLUMN [ IF NOT EXISTS ] This form adds a new column to the table, using the same syntax as - . + . If IF NOT EXISTS + is specified and the column already exists, no error is thrown. diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1c7eded..05fbe51 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -328,8 +328,9 @@ 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, @@ -2304,7 +2305,7 @@ renameatt_internal(Oid myrelid, oldattname))); /* new name should not already exist */ - check_for_column_name_collision(targetrelation, newattname); + (void) check_for_column_name_collision(targetrelation, newattname, false); /* apply the update */ namestrcpy(&(attform->attname), newattname); @@ -3455,11 +3456,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); @@ -3572,14 +3573,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);
Re: [HACKERS] Add CINE for ALTER TABLE ... ADD COLUMN
On Fri, Jun 26, 2015 at 12:41 AM, Fabrízio de Royes Mello wrote: > > > On Wed, Jun 24, 2015 at 3:36 PM, Alvaro Herrera > wrote: >> >> Fabrízio de Royes Mello wrote: >> >> > Another rebased version. >> >> There are a number of unrelated whitespace changes in this patch; also >> please update the comment on top of check_for_column_name_collision. >> > > Sorry, bad merging after a pgident run. Comments on top of > check_for_column_name collision also updated. I had a look at this patch, and here are some minor comments: 1) In alter_table.sgml, you need a space here: [ IF NOT EXISTS ]colname); + if (!check_for_column_name_collision(rel, colDef->colname, if_not_exists)) The new name can actually exist if if_not_exists is true. Except that the implementation looks sane to me. Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add CINE for ALTER TABLE ... ADD COLUMN
On Wed, Jun 24, 2015 at 3:36 PM, Alvaro Herrera
wrote:
>
> Fabrízio de Royes Mello wrote:
>
> > Another rebased version.
>
> There are a number of unrelated whitespace changes in this patch; also
> please update the comment on top of check_for_column_name_collision.
>
Sorry, bad merging after a pgident run. Comments on top of
check_for_column_name collision also updated.
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 name
where action is one of:
-ADD [ COLUMN ] column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ]
+ADD [ COLUMN ] [ IF NOT EXISTS ]column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ]
DROP [ COLUMN ] [ IF EXISTS ] column_name [ RESTRICT | CASCADE ]
ALTER [ COLUMN ] column_name [ SET DATA ] TYPE data_type [ COLLATE collation ] [ USING expression ]
ALTER [ COLUMN ] column_name SET DEFAULT expression
@@ -96,11 +96,12 @@ ALTER TABLE ALL IN TABLESPACE name
-ADD COLUMN
+ADD COLUMN [ IF NOT EXISTS ]
This form adds a new column to the table, using the same syntax as
- .
+ . If IF NOT EXISTS
+ is specified and the column already exists, no error is thrown.
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d394713..94791e6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -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,
@@ -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);
@@ -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 */
@@ -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
Re: [HACKERS] Add CINE for ALTER TABLE ... ADD COLUMN
Fabrízio de Royes Mello wrote: > Another rebased version. There are a number of unrelated whitespace changes in this patch; also please update the comment on top of check_for_column_name_collision. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add CINE for ALTER TABLE ... ADD COLUMN
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 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 name where action is one of: -ADD [ COLUMN ] column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ] +ADD [ COLUMN ] [ IF NOT EXISTS ]column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ] DROP [ COLUMN ] [ IF EXISTS ] column_name [ RESTRICT | CASCADE ] ALTER [ COLUMN ] column_name [ SET DATA ] TYPE data_type [ COLLATE collation ] [ USING expression ] ALTER [ COLUMN ] column_name SET DEFAULT expression @@ -96,11 +96,12 @@ ALTER TABLE ALL IN TABLESPACE name -ADD COLUMN +ADD COLUMN [ IF NOT EXISTS ] This form adds a new column to the table, using the same syntax as - . + . If IF NOT EXISTS + is specified and the column already exists, no error is thrown. 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 *)
Re: [HACKERS] Add CINE for ALTER TABLE ... ADD COLUMN
On Wed, Apr 22, 2015 at 3:48 PM, Payal Singh 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! 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 6a82730..3041b09 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 name where action is one of: -ADD [ COLUMN ] column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ] +ADD [ COLUMN ] [ IF NOT EXISTS ]column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ] DROP [ COLUMN ] [ IF EXISTS ] column_name [ RESTRICT | CASCADE ] ALTER [ COLUMN ] column_name [ SET DATA ] TYPE data_type [ COLLATE collation ] [ USING expression ] ALTER [ COLUMN ] column_name SET DEFAULT expression @@ -96,11 +96,12 @@ ALTER TABLE ALL IN TABLESPACE name -ADD COLUMN +ADD COLUMN [ IF NOT EXISTS ] This form adds a new column to the table, using the same syntax as - . + . If IF NOT EXISTS + is specified and the column already exists, no error is thrown. diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 06e4332..8cd436d 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -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, @@ -2294,7 +2294,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); @@ -3443,11 +3443,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); @@ -3490,19 +3490,19 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel, break; case AT_AddConstraint: /* ADD CONSTRAINT */ address = -ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, - false, false, lockmode); + ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, +false, false, lockmode); break; case AT_AddConstraintRecurse: /* ADD CONSTRAINT with recursion */ address = -ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, - true, false, lockmode); + ATExecAddConstraint(wqueue, tab, rel, (Constraint *) cmd->def, +true, false, lockmode); break; case AT_ReAddConstraint: /* Re-add p
Re: [HACKERS] Add CINE for ALTER TABLE ... ADD COLUMN
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Add CINE for ALTER TABLE ... ADD COLUMN
Hi all,
This simple patch add CINE for ALTER TABLE ... ADD COLUMN.
So now we can:
ALTER TABLE foo
ADD COLUMN IF NOT EXISTS c1 integer;
and/or ...
ALTER TABLE foo
ADD COLUMN IF NOT EXISTS c1 integer,
ADD COLUMN c2 integer;
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 b3a4970..aba7ec0 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 name
where action is one of:
-ADD [ COLUMN ] column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ]
+ADD [ COLUMN ] [ IF NOT EXISTS ]column_name data_type [ COLLATE collation ] [ column_constraint [ ... ] ]
DROP [ COLUMN ] [ IF EXISTS ] column_name [ RESTRICT | CASCADE ]
ALTER [ COLUMN ] column_name [ SET DATA ] TYPE data_type [ COLLATE collation ] [ USING expression ]
ALTER [ COLUMN ] column_name SET DEFAULT expression
@@ -96,11 +96,12 @@ ALTER TABLE ALL IN TABLESPACE name
-ADD COLUMN
+ADD COLUMN [ IF NOT EXISTS ]
This form adds a new column to the table, using the same syntax as
- .
+ . If IF NOT EXISTS
+ is specified and the column already exists, no error is thrown.
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f5d5b63..5ecb438 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -328,8 +328,8 @@ static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recu
AlterTableCmd *cmd, LOCKMODE lockmode);
static void 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,
@@ -2283,7 +2283,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);
@@ -3399,11 +3399,11 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
case AT_AddColumnToView: /* add column via CREATE OR REPLACE
* VIEW */
ATExecAddColumn(wqueue, tab, rel, (ColumnDef *) cmd->def,
- false, false, false, lockmode);
+ false, false, false, cmd->missing_ok, lockmode);
break;
case AT_AddColumnRecurse:
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 */
ATExecColumnDefault(rel, cmd->name, cmd->def, lockmode);
@@ -3500,13 +3500,13 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
/* Use the ADD COLUMN code, unless prep decided to do nothing */
if (cmd->def != NULL)
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)
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 */
@@ -4593,7 +4593,7 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
static void
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,
@@ -4687,7 +4687,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))
+ {
+ h
