Skip to content

Commit

Permalink
[#6316] ALTER TABLE .. ADD COLUMN ignores column type's DEFAULT clause
Browse files Browse the repository at this point in the history
  • Loading branch information
lukaseder committed Jun 5, 2017
1 parent 43ff705 commit ef55ac7
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 55 deletions.
12 changes: 2 additions & 10 deletions jOOQ/src/main/java/org/jooq/impl/AlterTableImpl.java
Expand Up @@ -76,7 +76,6 @@
import static org.jooq.impl.Keywords.K_IF_EXISTS; import static org.jooq.impl.Keywords.K_IF_EXISTS;
import static org.jooq.impl.Keywords.K_MODIFY; import static org.jooq.impl.Keywords.K_MODIFY;
import static org.jooq.impl.Keywords.K_NOT_NULL; import static org.jooq.impl.Keywords.K_NOT_NULL;
import static org.jooq.impl.Keywords.K_NULL;
import static org.jooq.impl.Keywords.K_RENAME_COLUMN; import static org.jooq.impl.Keywords.K_RENAME_COLUMN;
import static org.jooq.impl.Keywords.K_RENAME_CONSTRAINT; import static org.jooq.impl.Keywords.K_RENAME_CONSTRAINT;
import static org.jooq.impl.Keywords.K_RENAME_INDEX; import static org.jooq.impl.Keywords.K_RENAME_INDEX;
Expand All @@ -88,6 +87,7 @@
import static org.jooq.impl.Keywords.K_TYPE; import static org.jooq.impl.Keywords.K_TYPE;
import static org.jooq.impl.Keywords.K_USING_INDEX; import static org.jooq.impl.Keywords.K_USING_INDEX;
import static org.jooq.impl.Tools.toSQLDDLTypeDeclaration; import static org.jooq.impl.Tools.toSQLDDLTypeDeclaration;
import static org.jooq.impl.Tools.toSQLDDLTypeDeclarationForAddition;
import static org.jooq.impl.Tools.DataKey.DATA_CONSTRAINT_REFERENCE; import static org.jooq.impl.Tools.DataKey.DATA_CONSTRAINT_REFERENCE;


import org.jooq.AlterTableAlterStep; import org.jooq.AlterTableAlterStep;
Expand Down Expand Up @@ -619,15 +619,7 @@ else if (addColumn != null) {
.visit(addColumn).sql(' ') .visit(addColumn).sql(' ')
.qualify(qualify); .qualify(qualify);


toSQLDDLTypeDeclaration(ctx, addColumnType); toSQLDDLTypeDeclarationForAddition(ctx, addColumnType);

if (!addColumnType.nullable())
ctx.sql(' ').visit(K_NOT_NULL);

// Some databases default to NOT NULL, so explicitly setting columns to NULL is mostly required here
// [#3400] [#4321] ... but not in Derby, Firebird
else if (!asList(DERBY, FIREBIRD).contains(family))
ctx.sql(' ').visit(K_NULL);






Expand Down
46 changes: 1 addition & 45 deletions jOOQ/src/main/java/org/jooq/impl/CreateTableImpl.java
Expand Up @@ -57,18 +57,12 @@
import static org.jooq.impl.DSL.insertInto; import static org.jooq.impl.DSL.insertInto;
import static org.jooq.impl.DSL.name; import static org.jooq.impl.DSL.name;
import static org.jooq.impl.Keywords.K_AS; import static org.jooq.impl.Keywords.K_AS;
import static org.jooq.impl.Keywords.K_AUTO_INCREMENT;
import static org.jooq.impl.Keywords.K_BEGIN; import static org.jooq.impl.Keywords.K_BEGIN;
import static org.jooq.impl.Keywords.K_CREATE; import static org.jooq.impl.Keywords.K_CREATE;
import static org.jooq.impl.Keywords.K_DEFAULT;
import static org.jooq.impl.Keywords.K_END; import static org.jooq.impl.Keywords.K_END;
import static org.jooq.impl.Keywords.K_EXECUTE_IMMEDIATE; import static org.jooq.impl.Keywords.K_EXECUTE_IMMEDIATE;
import static org.jooq.impl.Keywords.K_GENERATED_BY_DEFAULT_AS_IDENTITY;
import static org.jooq.impl.Keywords.K_GLOBAL_TEMPORARY; import static org.jooq.impl.Keywords.K_GLOBAL_TEMPORARY;
import static org.jooq.impl.Keywords.K_IDENTITY;
import static org.jooq.impl.Keywords.K_IF_NOT_EXISTS; import static org.jooq.impl.Keywords.K_IF_NOT_EXISTS;
import static org.jooq.impl.Keywords.K_NOT_NULL;
import static org.jooq.impl.Keywords.K_NULL;
import static org.jooq.impl.Keywords.K_ON_COMMIT_DELETE_ROWS; import static org.jooq.impl.Keywords.K_ON_COMMIT_DELETE_ROWS;
import static org.jooq.impl.Keywords.K_ON_COMMIT_DROP; import static org.jooq.impl.Keywords.K_ON_COMMIT_DROP;
import static org.jooq.impl.Keywords.K_ON_COMMIT_PRESERVE_ROWS; import static org.jooq.impl.Keywords.K_ON_COMMIT_PRESERVE_ROWS;
Expand Down Expand Up @@ -269,40 +263,7 @@ private final void accept0(Context<?> ctx) {


ctx.visit(columnFields.get(i)) ctx.visit(columnFields.get(i))
.sql(' '); .sql(' ');
Tools.toSQLDDLTypeDeclaration(ctx, type); Tools.toSQLDDLTypeDeclarationForAddition(ctx, type);

// [#5356] Some dialects require the DEFAULT clause prior to the
// NULL constraints clause
if (asList(HSQLDB).contains(ctx.family()))
acceptDefault(ctx, type);

if (type.nullable()) {

// [#4321] Not all dialects support explicit NULL type declarations
if (!asList(DERBY, FIREBIRD).contains(ctx.family()))
ctx.sql(' ').visit(K_NULL);
}
else {
ctx.sql(' ').visit(K_NOT_NULL);
}

if (type.identity()) {

// [#5062] H2's (and others') AUTO_INCREMENT flag is syntactically located *after* NULL flags.
switch (ctx.family()) {





case H2:
case MARIADB:
case MYSQL: ctx.sql(' ').visit(K_AUTO_INCREMENT); break;
}
}

if (!asList(HSQLDB).contains(ctx.family()))
acceptDefault(ctx, type);


if (i < columnFields.size() - 1) if (i < columnFields.size() - 1)
ctx.sql(',').formatSeparator(); ctx.sql(',').formatSeparator();
Expand All @@ -328,11 +289,6 @@ private final void accept0(Context<?> ctx) {
} }
} }


private final void acceptDefault(Context<?> ctx, DataType<?> type) {
if (type.defaulted())
ctx.sql(' ').visit(K_DEFAULT).sql(' ').visit(type.defaultValue());
}

private final void acceptCreateTableAsSelect(Context<?> ctx) { private final void acceptCreateTableAsSelect(Context<?> ctx) {
ctx.start(CREATE_TABLE); ctx.start(CREATE_TABLE);
toSQLCreateTableName(ctx); toSQLCreateTableName(ctx);
Expand Down
46 changes: 46 additions & 0 deletions jOOQ/src/main/java/org/jooq/impl/Tools.java
Expand Up @@ -40,6 +40,9 @@
// ... // ...
// ... // ...
import static org.jooq.SQLDialect.CUBRID; import static org.jooq.SQLDialect.CUBRID;
import static org.jooq.SQLDialect.DERBY;
import static org.jooq.SQLDialect.FIREBIRD;
import static org.jooq.SQLDialect.HSQLDB;
import static org.jooq.SQLDialect.MARIADB; import static org.jooq.SQLDialect.MARIADB;
import static org.jooq.SQLDialect.MYSQL; import static org.jooq.SQLDialect.MYSQL;
// ... // ...
Expand Down Expand Up @@ -84,6 +87,7 @@
import static org.jooq.impl.Keywords.K_BEGIN_CATCH; import static org.jooq.impl.Keywords.K_BEGIN_CATCH;
import static org.jooq.impl.Keywords.K_BEGIN_TRY; import static org.jooq.impl.Keywords.K_BEGIN_TRY;
import static org.jooq.impl.Keywords.K_DECLARE; import static org.jooq.impl.Keywords.K_DECLARE;
import static org.jooq.impl.Keywords.K_DEFAULT;
import static org.jooq.impl.Keywords.K_DO; import static org.jooq.impl.Keywords.K_DO;
import static org.jooq.impl.Keywords.K_ELSE; import static org.jooq.impl.Keywords.K_ELSE;
import static org.jooq.impl.Keywords.K_END; import static org.jooq.impl.Keywords.K_END;
Expand All @@ -105,6 +109,7 @@
import static org.jooq.impl.Keywords.K_INT; import static org.jooq.impl.Keywords.K_INT;
import static org.jooq.impl.Keywords.K_LIKE; import static org.jooq.impl.Keywords.K_LIKE;
import static org.jooq.impl.Keywords.K_LOOP; import static org.jooq.impl.Keywords.K_LOOP;
import static org.jooq.impl.Keywords.K_NOT_NULL;
import static org.jooq.impl.Keywords.K_NULL; import static org.jooq.impl.Keywords.K_NULL;
import static org.jooq.impl.Keywords.K_NVARCHAR; import static org.jooq.impl.Keywords.K_NVARCHAR;
import static org.jooq.impl.Keywords.K_RAISE; import static org.jooq.impl.Keywords.K_RAISE;
Expand Down Expand Up @@ -3529,6 +3534,47 @@ static final void executeImmediateIfExistsEnd(Context<?> ctx, DDLStatementType t
} }
} }


static final void toSQLDDLTypeDeclarationForAddition(Context<?> ctx, DataType<?> type) {
toSQLDDLTypeDeclaration(ctx, type);

// [#5356] Some dialects require the DEFAULT clause prior to the
// NULL constraints clause
if (asList(HSQLDB).contains(ctx.family()))
toSQLDDLTypeDeclarationDefault(ctx, type);

if (!type.nullable())
ctx.sql(' ').visit(K_NOT_NULL);

// Some databases default to NOT NULL, so explicitly setting columns to NULL is mostly required here
// [#3400] [#4321] ... but not in Derby, Firebird
else if (!asList(DERBY, FIREBIRD).contains(ctx.family()))
ctx.sql(' ').visit(K_NULL);

if (type.identity()) {

// [#5062] H2's (and others') AUTO_INCREMENT flag is syntactically located *after* NULL flags.
switch (ctx.family()) {





case H2:
case MARIADB:
case MYSQL: ctx.sql(' ').visit(K_AUTO_INCREMENT); break;
}
}

if (!asList(HSQLDB).contains(ctx.family()))
toSQLDDLTypeDeclarationDefault(ctx, type);
}

private static final void toSQLDDLTypeDeclarationDefault(Context<?> ctx, DataType<?> type) {
if (type.defaulted())
ctx.sql(' ').visit(K_DEFAULT).sql(' ').visit(type.defaultValue());
}


static final void toSQLDDLTypeDeclaration(Context<?> ctx, DataType<?> type) { static final void toSQLDDLTypeDeclaration(Context<?> ctx, DataType<?> type) {
String typeName = type.getTypeName(ctx.configuration()); String typeName = type.getTypeName(ctx.configuration());


Expand Down

0 comments on commit ef55ac7

Please sign in to comment.