This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new bdb0b2ad3e [Cherry-pick to branch-1.3] [#14056] fix(catalog-jdbc):
Allow changing nullable column without default to NOT NULL (#14057) (#14195)
bdb0b2ad3e is described below
commit bdb0b2ad3e842af2f10730d33585c6126257db19
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Oct 26 14:49:48 2026 +0800
[Cherry-pick to branch-1.3] [#14056] fix(catalog-jdbc): Allow changing
nullable column without default to NOT NULL (#14057) (#14195)
**Cherry-pick Information:**
- Original commit: 993ed4038aeae8b5e3e30b6eda4f7293f6aa7d84
- Target branch: `branch-1.3`
- Status: ✅ Conflicts resolved
---------
Co-authored-by: Yuhui <[email protected]>
Co-authored-by: Jerry Shao <[email protected]>
Co-authored-by: diqiu50 <[email protected]>
---
.../operations/ClickHouseTableOperations.java | 33 ++++-------
.../operations/TestClickHouseTableOperations.java | 43 ++++++++++----
.../operation/OceanBaseTableOperations.java | 34 ++++-------
.../operation/TestOceanBaseTableOperations.java | 22 ++++---
.../jdbc/operation/JdbcTableOperations.java | 65 +++++++++++++++++++--
.../jdbc/operation/TestJdbcTableOperations.java | 65 +++++++++++++++++++++
.../doris/operation/DorisTableOperations.java | 8 ++-
.../TestDorisTableOperationsSqlGeneration.java | 35 +++++++++--
.../mysql/operation/MysqlTableOperations.java | 34 ++++-------
.../mysql/operation/TestMysqlTableOperations.java | 22 ++++---
.../operation/PostgreSqlTableOperations.java | 20 ++++++-
.../operation/TestPostgreSqlTableOperations.java | 67 ++++++++++++++++++++++
12 files changed, 337 insertions(+), 111 deletions(-)
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
index 2f7322bda4..92564dcf05 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/main/java/org/apache/gravitino/catalog/clickhouse/operations/ClickHouseTableOperations.java
@@ -1080,7 +1080,8 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
} else if (change instanceof TableChange.UpdateColumnDefaultValue
updateColumnDefaultValue) {
lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
alterSql.add(
- updateColumnDefaultValueFieldDefinition(updateColumnDefaultValue,
lazyLoadTable));
+ updateColumnDefaultValueFieldDefinition(
+ updateColumnDefaultValue, lazyLoadTable, changes));
} else if (change instanceof TableChange.UpdateColumnType
updateColumnType) {
lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
@@ -1106,7 +1107,7 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
alterSql.add(
updateColumnNullabilityDefinition(
- (TableChange.UpdateColumnNullability) change, lazyLoadTable));
+ (TableChange.UpdateColumnNullability) change, lazyLoadTable,
changes));
} else if (change instanceof TableChange.AddIndex addIndex) {
lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
@@ -1294,20 +1295,11 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
}
private String updateColumnNullabilityDefinition(
- TableChange.UpdateColumnNullability change, JdbcTable table) {
- validateUpdateColumnNullable(change, table);
+ TableChange.UpdateColumnNullability change, JdbcTable table,
TableChange... changes) {
+ JdbcColumn column = validateUpdateColumnNullable(change, table);
String col = change.fieldName()[0];
- JdbcColumn column = getJdbcColumnFromTable(table, col);
- JdbcColumn updateColumn =
- JdbcColumn.builder()
- .withName(col)
- .withDefaultValue(column.defaultValue())
- .withNullable(change.nullable())
- .withType(column.dataType())
- .withComment(column.comment())
- .withAutoIncrement(column.autoIncrement())
- .build();
+ JdbcColumn updateColumn = withPendingDefaultValueAndNullability(column,
changes);
return "%s %s %s"
.formatted(
@@ -1460,7 +1452,9 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
}
private String updateColumnDefaultValueFieldDefinition(
- TableChange.UpdateColumnDefaultValue updateColumnDefaultValue, JdbcTable
jdbcTable) {
+ TableChange.UpdateColumnDefaultValue updateColumnDefaultValue,
+ JdbcTable jdbcTable,
+ TableChange... changes) {
if (updateColumnDefaultValue.fieldName().length > 1) {
throw new
UnsupportedOperationException(CLICKHOUSE_NOT_SUPPORT_NESTED_COLUMN_MSG);
}
@@ -1468,14 +1462,7 @@ public class ClickHouseTableOperations extends
JdbcTableOperations {
String col = updateColumnDefaultValue.fieldName()[0];
JdbcColumn column = getJdbcColumnFromTable(jdbcTable, col);
StringBuilder sqlBuilder = new StringBuilder(MODIFY_COLUMN +
quoteIdentifier(col));
- JdbcColumn newColumn =
- JdbcColumn.builder()
- .withName(col)
- .withType(column.dataType())
- .withNullable(column.nullable())
- .withComment(column.comment())
- .withDefaultValue(updateColumnDefaultValue.getNewDefaultValue())
- .build();
+ JdbcColumn newColumn = withPendingDefaultValueAndNullability(column,
changes);
return appendColumnDefinition(newColumn, sqlBuilder).toString();
}
diff --git
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java
index 8967a19636..c184258ba5 100644
---
a/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-clickhouse/src/test/java/org/apache/gravitino/catalog/clickhouse/operations/TestClickHouseTableOperations.java
@@ -1783,7 +1783,7 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
}
@Test
- public void testAlterTableNullabilityValidationFails() {
+ public void testAlterTableNullabilityDropsNullDefault() {
StubClickHouseTableOperations ops = new StubClickHouseTableOperations();
ops.initialize(
null,
@@ -1793,15 +1793,38 @@ public class TestClickHouseTableOperations extends
TestClickHouse {
new HashMap<>());
ops.setTable(buildStubTableWithNullDefault());
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () ->
- ops.buildAlterSql(
- "db",
- "tbl",
- new TableChange[] {
- TableChange.updateColumnNullability(new String[] {"c3"},
false)
- }));
+ String sql =
+ ops.buildAlterSql(
+ "db",
+ "tbl",
+ new TableChange[] {TableChange.updateColumnNullability(new
String[] {"c3"}, false)});
+ Assertions.assertTrue(sql.replaceAll("\\s+", " ").contains("MODIFY COLUMN
`c3` String"), sql);
+ Assertions.assertFalse(sql.contains("Nullable"), sql);
+ Assertions.assertFalse(sql.contains("DEFAULT"), sql);
+ }
+
+ @Test
+ public void testAlterTableNullabilityWithDefaultValueInSameRequest() {
+ StubClickHouseTableOperations ops = new StubClickHouseTableOperations();
+ ops.initialize(
+ null,
+ new ClickHouseExceptionConverter(),
+ new ClickHouseTypeConverter(),
+ new ClickHouseColumnDefaultValueConverter(),
+ new HashMap<>());
+ ops.setTable(buildStubTableWithNullDefault());
+
+ TableChange setDefault =
+ TableChange.updateColumnDefaultValue(new String[] {"c3"},
Literals.stringLiteral("abc"));
+ TableChange setNotNull = TableChange.updateColumnNullability(new String[]
{"c3"}, false);
+ // Every regenerated column definition carries both the new default and
NOT NULL, whichever
+ // order the changes come in.
+ for (TableChange[] changes :
+ new TableChange[][] {{setDefault, setNotNull}, {setNotNull,
setDefault}}) {
+ String sql = ops.buildAlterSql("db", "tbl", changes);
+ Assertions.assertFalse(sql.contains("Nullable"), sql);
+ Assertions.assertEquals(2, StringUtils.countMatches(sql, "DEFAULT
'abc'"), sql);
+ }
}
@Test
diff --git
a/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
b/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
index 99b0a85b1c..799affe6e7 100644
---
a/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-oceanbase/src/main/java/org/apache/gravitino/catalog/oceanbase/operation/OceanBaseTableOperations.java
@@ -257,7 +257,8 @@ public class OceanBaseTableOperations extends
JdbcTableOperations {
TableChange.UpdateColumnDefaultValue updateColumnDefaultValue =
(TableChange.UpdateColumnDefaultValue) change;
alterSql.add(
- updateColumnDefaultValueFieldDefinition(updateColumnDefaultValue,
lazyLoadTable));
+ updateColumnDefaultValueFieldDefinition(
+ updateColumnDefaultValue, lazyLoadTable, changes));
} else if (change instanceof TableChange.UpdateColumnType) {
lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
TableChange.UpdateColumnType updateColumnType =
(TableChange.UpdateColumnType) change;
@@ -283,7 +284,7 @@ public class OceanBaseTableOperations extends
JdbcTableOperations {
lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
alterSql.add(
updateColumnNullabilityDefinition(
- (TableChange.UpdateColumnNullability) change, lazyLoadTable));
+ (TableChange.UpdateColumnNullability) change, lazyLoadTable,
changes));
} else if (change instanceof TableChange.AddIndex) {
alterSql.add(addIndexDefinition((TableChange.AddIndex) change));
} else if (change instanceof TableChange.DeleteIndex) {
@@ -368,19 +369,10 @@ public class OceanBaseTableOperations extends
JdbcTableOperations {
}
private String updateColumnNullabilityDefinition(
- TableChange.UpdateColumnNullability change, JdbcTable table) {
- validateUpdateColumnNullable(change, table);
+ TableChange.UpdateColumnNullability change, JdbcTable table,
TableChange... changes) {
+ JdbcColumn column = validateUpdateColumnNullable(change, table);
String col = change.fieldName()[0];
- JdbcColumn column = getJdbcColumnFromTable(table, col);
- JdbcColumn updateColumn =
- JdbcColumn.builder()
- .withName(col)
- .withDefaultValue(column.defaultValue())
- .withNullable(change.nullable())
- .withType(column.dataType())
- .withComment(column.comment())
- .withAutoIncrement(column.autoIncrement())
- .build();
+ JdbcColumn updateColumn = withPendingDefaultValueAndNullability(column,
changes);
return MODIFY_COLUMN
+ BACK_QUOTE
+ col
@@ -591,22 +583,16 @@ public class OceanBaseTableOperations extends
JdbcTableOperations {
}
private String updateColumnDefaultValueFieldDefinition(
- TableChange.UpdateColumnDefaultValue updateColumnDefaultValue, JdbcTable
jdbcTable) {
+ TableChange.UpdateColumnDefaultValue updateColumnDefaultValue,
+ JdbcTable jdbcTable,
+ TableChange... changes) {
if (updateColumnDefaultValue.fieldName().length > 1) {
throw new
UnsupportedOperationException(OCEANBASE_NOT_SUPPORT_NESTED_COLUMN_MSG);
}
String col = updateColumnDefaultValue.fieldName()[0];
JdbcColumn column = getJdbcColumnFromTable(jdbcTable, col);
StringBuilder sqlBuilder = new StringBuilder(MODIFY_COLUMN + BACK_QUOTE +
col + BACK_QUOTE);
- JdbcColumn newColumn =
- JdbcColumn.builder()
- .withName(col)
- .withType(column.dataType())
- .withNullable(column.nullable())
- .withComment(column.comment())
- .withAutoIncrement(column.autoIncrement())
- .withDefaultValue(updateColumnDefaultValue.getNewDefaultValue())
- .build();
+ JdbcColumn newColumn = withPendingDefaultValueAndNullability(column,
changes);
return appendColumnDefinition(newColumn, sqlBuilder).toString();
}
diff --git
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
index 3a07734d6c..237b4e378a 100644
---
a/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
+++
b/catalogs-contrib/catalog-jdbc-oceanbase/src/test/java/org/apache/gravitino/catalog/oceanbase/operation/TestOceanBaseTableOperations.java
@@ -550,14 +550,20 @@ public class TestOceanBaseTableOperations extends
TestOceanBase {
assertionsTableInfo(
tableName, newComment, columns, properties, indexes,
Transforms.EMPTY_TRANSFORM, load);
- TableChange updateColumn =
- TableChange.updateColumnNullability(new String[] {col3.name()},
!col3.nullable());
- IllegalArgumentException exception =
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () -> TABLE_OPERATIONS.alterTable(TEST_DB_NAME, tableName,
updateColumn));
- Assertions.assertTrue(
- exception.getMessage().contains("with null default value cannot be
changed to not null"));
+ // A nullable column whose default is NULL can be changed to not null.
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME,
+ tableName,
+ TableChange.updateColumnNullability(new String[] {col3.name()},
false));
+ load = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName);
+ JdbcColumn updatedCol3 =
+ (JdbcColumn)
+ Arrays.stream(load.columns())
+ .filter(c -> c.name().equals(col3.name()))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("Column not found: " +
col3.name()));
+ Assertions.assertFalse(updatedCol3.nullable());
+ Assertions.assertEquals(Column.DEFAULT_VALUE_NOT_SET,
updatedCol3.defaultValue());
}
@Test
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcTableOperations.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcTableOperations.java
index 7f9b9cfa14..98b54eef43 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcTableOperations.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/operation/JdbcTableOperations.java
@@ -587,17 +587,70 @@ public abstract class JdbcTableOperations implements
TableOperation {
return null != lazyLoadCreateTable ? lazyLoadCreateTable :
load(databaseName, tableName);
}
- protected void validateUpdateColumnNullable(
+ /**
+ * Validates a nullability change and returns the column it applies to.
+ *
+ * @param change the nullability change
+ * @param table the table that contains the column
+ * @return the current column
+ */
+ protected JdbcColumn validateUpdateColumnNullable(
TableChange.UpdateColumnNullability change, JdbcTable table) {
if (change.fieldName().length > 1) {
throw new UnsupportedOperationException("Nested column names are not
supported");
}
- String col = change.fieldName()[0];
- JdbcColumn column = getJdbcColumnFromTable(table, col);
- if (!change.nullable() && column.defaultValue().equals(Literals.NULL)) {
- throw new IllegalArgumentException(
- "column " + col + " with null default value cannot be changed to not
null");
+ return getJdbcColumnFromTable(table, change.fieldName()[0]);
+ }
+
+ /**
+ * Returns a copy of the column with the default value and nullability it
has after all default
+ * value and nullability changes to it in the same alter request are
applied. Dialects that
+ * regenerate the whole column definition for one change use it, so that the
regenerated
+ * definition does not revert another change to the same column. A NULL
default conflicts with NOT
+ * NULL: the existing NULL default is dropped when the column ends up
non-nullable, while a NULL
+ * default requested in the same alter request is rejected.
+ *
+ * @param column the current column
+ * @param changes all changes in the alter request
+ * @return the column with the pending default value and nullability
+ * @throws IllegalArgumentException if the request sets a NULL default on a
column that ends up
+ * non-nullable
+ */
+ protected static JdbcColumn withPendingDefaultValueAndNullability(
+ JdbcColumn column, TableChange... changes) {
+ Expression defaultValue = column.defaultValue();
+ boolean defaultValueChanged = false;
+ boolean nullable = column.nullable();
+ for (TableChange change : changes) {
+ if (!(change instanceof TableChange.ColumnChange)) {
+ continue;
+ }
+ String[] fieldName = ((TableChange.ColumnChange) change).fieldName();
+ if (fieldName.length != 1 || !fieldName[0].equals(column.name())) {
+ continue;
+ }
+ if (change instanceof TableChange.UpdateColumnDefaultValue) {
+ defaultValue = ((TableChange.UpdateColumnDefaultValue)
change).getNewDefaultValue();
+ defaultValueChanged = true;
+ } else if (change instanceof TableChange.UpdateColumnNullability) {
+ nullable = ((TableChange.UpdateColumnNullability) change).nullable();
+ }
}
+ if (!nullable && Literals.NULL.equals(defaultValue)) {
+ Preconditions.checkArgument(
+ !defaultValueChanged,
+ "Column %s cannot have a NULL default value because it is not
nullable",
+ column.name());
+ defaultValue = DEFAULT_VALUE_NOT_SET;
+ }
+ return JdbcColumn.builder()
+ .withName(column.name())
+ .withType(column.dataType())
+ .withComment(column.comment())
+ .withNullable(nullable)
+ .withAutoIncrement(column.autoIncrement())
+ .withDefaultValue(defaultValue)
+ .build();
}
/**
diff --git
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/operation/TestJdbcTableOperations.java
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/operation/TestJdbcTableOperations.java
index ff8fcf8bb7..9fa0549001 100644
---
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/operation/TestJdbcTableOperations.java
+++
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/operation/TestJdbcTableOperations.java
@@ -46,6 +46,7 @@ import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
+import org.apache.gravitino.rel.expressions.literals.Literals;
import org.apache.gravitino.rel.indexes.Indexes;
import org.apache.gravitino.rel.types.Type;
import org.apache.gravitino.rel.types.Types;
@@ -245,6 +246,70 @@ public class TestJdbcTableOperations {
Assertions.assertTrue(JDBC_TABLE_OPERATIONS.isMySQLDriverVersionSupported("org.sqlite.JDBC"));
}
+ @Test
+ public void testWithPendingDefaultValueAndNullability() {
+ JdbcColumn column =
+ JdbcColumn.builder()
+ .withName("col_1")
+ .withType(Types.IntegerType.get())
+ .withComment("comment")
+ .withNullable(true)
+ .withDefaultValue(Literals.NULL)
+ .build();
+ String[] fieldName = {"col_1"};
+
+ // A NULL default is dropped when the column becomes not null.
+ JdbcColumn pending =
+ JdbcTableOperations.withPendingDefaultValueAndNullability(
+ column, TableChange.updateColumnNullability(fieldName, false));
+ Assertions.assertFalse(pending.nullable());
+ Assertions.assertEquals(Column.DEFAULT_VALUE_NOT_SET,
pending.defaultValue());
+ Assertions.assertEquals("comment", pending.comment());
+ Assertions.assertEquals(Types.IntegerType.get(), pending.dataType());
+
+ // A new default in the same request is kept, whichever order the changes
come in.
+ TableChange setDefault =
+ TableChange.updateColumnDefaultValue(fieldName,
Literals.integerLiteral(5));
+ TableChange setNotNull = TableChange.updateColumnNullability(fieldName,
false);
+ for (TableChange[] changes :
+ new TableChange[][] {{setDefault, setNotNull}, {setNotNull,
setDefault}}) {
+ pending =
JdbcTableOperations.withPendingDefaultValueAndNullability(column, changes);
+ Assertions.assertFalse(pending.nullable());
+ Assertions.assertEquals(Literals.integerLiteral(5),
pending.defaultValue());
+ }
+
+ // Changes to other columns are ignored.
+ pending =
+ JdbcTableOperations.withPendingDefaultValueAndNullability(
+ column,
+ TableChange.updateColumnNullability(new String[] {"col_2"}, false),
+ TableChange.updateColumnDefaultValue(
+ new String[] {"col_2"}, Literals.integerLiteral(5)));
+ Assertions.assertTrue(pending.nullable());
+ Assertions.assertEquals(Literals.NULL, pending.defaultValue());
+
+ // A NULL default requested for a column that ends up not null is rejected.
+ TableChange setNullDefault =
TableChange.updateColumnDefaultValue(fieldName, Literals.NULL);
+ for (TableChange[] changes :
+ new TableChange[][] {{setNullDefault, setNotNull}, {setNotNull,
setNullDefault}}) {
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
JdbcTableOperations.withPendingDefaultValueAndNullability(column, changes));
+ }
+ JdbcColumn notNullColumn =
+ JdbcColumn.builder()
+ .withName("col_1")
+ .withType(Types.IntegerType.get())
+ .withNullable(false)
+ .withDefaultValue(Literals.integerLiteral(5))
+ .build();
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ JdbcTableOperations.withPendingDefaultValueAndNullability(
+ notNullColumn, setNullDefault));
+ }
+
private static JdbcColumn[] generateRandomColumn(int minSize, int maxSize) {
Random r = new Random();
String prefixColName = "col_";
diff --git
a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java
b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java
index b3627a50b1..24ce6ae61f 100644
---
a/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java
+++
b/catalogs/catalog-jdbc-doris/src/main/java/org/apache/gravitino/catalog/doris/operation/DorisTableOperations.java
@@ -956,9 +956,13 @@ public class DorisTableOperations extends
JdbcTableOperations {
private String updateColumnNullabilityDefinition(
TableChange.UpdateColumnNullability change, JdbcTable table,
Optional<String> dorisVersion) {
- validateUpdateColumnNullable(change, table);
+ JdbcColumn column = validateUpdateColumnNullable(change, table);
String col = change.fieldName()[0];
- JdbcColumn column = getJdbcColumnFromTable(table, col);
+ // Doris schema change does not allow converting a nullable column to
non-nullable.
+ Preconditions.checkArgument(
+ change.nullable() || !column.nullable(),
+ "Doris does not support changing nullable column %s to not null",
+ col);
JdbcColumn updateColumn =
JdbcColumn.builder()
.withName(col)
diff --git
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
index 79f6d5dfbe..d08fcf4c7f 100644
---
a/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
+++
b/catalogs/catalog-jdbc-doris/src/test/java/org/apache/gravitino/catalog/doris/operation/TestDorisTableOperationsSqlGeneration.java
@@ -147,6 +147,31 @@ public class TestDorisTableOperationsSqlGeneration {
}
}
+ /** Verifies a nullable column cannot be changed to not null. */
+ @Test
+ public void testUpdateColumnNullabilityRejectsNullableToNotNull() {
+ TestableDorisTableOperations ops = new TestableDorisTableOperations();
+ JdbcColumn column =
+ JdbcColumn.builder()
+ .withName("col1")
+ .withType(Types.IntegerType.get())
+ .withNullable(true)
+ .withDefaultValue(Literals.NULL)
+ .build();
+ ops.setTableForAlter(
+ JdbcTable.builder().withName("test_table").withColumns(new
JdbcColumn[] {column}).build());
+
+ IllegalArgumentException exception =
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ ops.alterTableSql(
+ "test_table",
+ TableChange.updateColumnNullability(new String[] {"col1"},
false)));
+ Assertions.assertEquals(
+ "Doris does not support changing nullable column col1 to not null",
exception.getMessage());
+ }
+
/** Verifies string defaults and unchanged column attributes are preserved.
*/
@Test
public void testUpdateColumnTypePreservesStringDefaultAndColumnAttributes() {
@@ -233,7 +258,7 @@ public class TestDorisTableOperationsSqlGeneration {
JdbcColumn.builder()
.withName("col1")
.withType(Types.VarCharType.of(64))
- .withNullable(true)
+ .withNullable(false)
.withComment("column comment")
.withDefaultValue(Literals.varcharLiteral(64, value))
.build();
@@ -246,7 +271,7 @@ public class TestDorisTableOperationsSqlGeneration {
String sql =
ops.alterTableSql(
"test_table",
- TableChange.updateColumnNullability(new String[] {"col1"},
false),
+ TableChange.updateColumnNullability(new String[] {"col1"}, true),
TableChange.updateColumnPosition(
new String[] {"col1"}, TableChange.ColumnPosition.first()),
TableChange.updateColumnType(new String[] {"col1"},
Types.VarCharType.of(128)));
@@ -257,13 +282,13 @@ public class TestDorisTableOperationsSqlGeneration {
.fromGravitinoForModifyColumn(column.defaultValue(), isDoris3x,
isDoris3x);
Assertions.assertEquals(
"ALTER TABLE `test_table`\n"
- + "MODIFY COLUMN `col1` varchar(64) NOT NULL DEFAULT "
+ + "MODIFY COLUMN `col1` varchar(64) NULL DEFAULT "
+ expectedDefault
+ " COMMENT 'column comment' ,\n"
- + "MODIFY COLUMN `col1` varchar(64) NULL DEFAULT "
+ + "MODIFY COLUMN `col1` varchar(64) NOT NULL DEFAULT "
+ expectedDefault
+ " COMMENT 'column comment' FIRST,\n"
- + "MODIFY COLUMN `col1` varchar(128) NULL DEFAULT "
+ + "MODIFY COLUMN `col1` varchar(128) NOT NULL DEFAULT "
+ expectedDefault
+ " COMMENT 'column comment' ;",
sql);
diff --git
a/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
b/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
index 5e755cb222..2b4a6d1f6c 100644
---
a/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
+++
b/catalogs/catalog-jdbc-mysql/src/main/java/org/apache/gravitino/catalog/mysql/operation/MysqlTableOperations.java
@@ -344,7 +344,8 @@ public class MysqlTableOperations extends
JdbcTableOperations {
TableChange.UpdateColumnDefaultValue updateColumnDefaultValue =
(TableChange.UpdateColumnDefaultValue) change;
alterSql.add(
- updateColumnDefaultValueFieldDefinition(updateColumnDefaultValue,
lazyLoadTable));
+ updateColumnDefaultValueFieldDefinition(
+ updateColumnDefaultValue, lazyLoadTable, changes));
} else if (change instanceof TableChange.UpdateColumnType) {
lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
TableChange.UpdateColumnType updateColumnType =
(TableChange.UpdateColumnType) change;
@@ -370,7 +371,7 @@ public class MysqlTableOperations extends
JdbcTableOperations {
lazyLoadTable = getOrCreateTable(databaseName, tableName,
lazyLoadTable);
alterSql.add(
updateColumnNullabilityDefinition(
- (TableChange.UpdateColumnNullability) change, lazyLoadTable));
+ (TableChange.UpdateColumnNullability) change, lazyLoadTable,
changes));
} else if (change instanceof TableChange.AddIndex) {
alterSql.add(addIndexDefinition((TableChange.AddIndex) change));
} else if (change instanceof TableChange.DeleteIndex) {
@@ -455,19 +456,10 @@ public class MysqlTableOperations extends
JdbcTableOperations {
}
private String updateColumnNullabilityDefinition(
- TableChange.UpdateColumnNullability change, JdbcTable table) {
- validateUpdateColumnNullable(change, table);
+ TableChange.UpdateColumnNullability change, JdbcTable table,
TableChange... changes) {
+ JdbcColumn column = validateUpdateColumnNullable(change, table);
String col = change.fieldName()[0];
- JdbcColumn column = getJdbcColumnFromTable(table, col);
- JdbcColumn updateColumn =
- JdbcColumn.builder()
- .withName(col)
- .withDefaultValue(column.defaultValue())
- .withNullable(change.nullable())
- .withType(column.dataType())
- .withComment(column.comment())
- .withAutoIncrement(column.autoIncrement())
- .build();
+ JdbcColumn updateColumn = withPendingDefaultValueAndNullability(column,
changes);
return MODIFY_COLUMN
+ BACK_QUOTE
+ col
@@ -686,22 +678,16 @@ public class MysqlTableOperations extends
JdbcTableOperations {
}
private String updateColumnDefaultValueFieldDefinition(
- TableChange.UpdateColumnDefaultValue updateColumnDefaultValue, JdbcTable
jdbcTable) {
+ TableChange.UpdateColumnDefaultValue updateColumnDefaultValue,
+ JdbcTable jdbcTable,
+ TableChange... changes) {
if (updateColumnDefaultValue.fieldName().length > 1) {
throw new
UnsupportedOperationException(MYSQL_NOT_SUPPORT_NESTED_COLUMN_MSG);
}
String col = updateColumnDefaultValue.fieldName()[0];
JdbcColumn column = getJdbcColumnFromTable(jdbcTable, col);
StringBuilder sqlBuilder = new StringBuilder(MODIFY_COLUMN + quote(col));
- JdbcColumn newColumn =
- JdbcColumn.builder()
- .withName(col)
- .withType(column.dataType())
- .withNullable(column.nullable())
- .withComment(column.comment())
- .withAutoIncrement(column.autoIncrement())
- .withDefaultValue(updateColumnDefaultValue.getNewDefaultValue())
- .build();
+ JdbcColumn newColumn = withPendingDefaultValueAndNullability(column,
changes);
return appendColumnDefinition(newColumn, sqlBuilder).toString();
}
diff --git
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
index dd32224d32..68d37eb09c 100644
---
a/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
+++
b/catalogs/catalog-jdbc-mysql/src/test/java/org/apache/gravitino/catalog/mysql/operation/TestMysqlTableOperations.java
@@ -521,14 +521,20 @@ public class TestMysqlTableOperations extends TestMysql {
assertionsTableInfo(
tableName, newComment, columns, properties, indexes,
Transforms.EMPTY_TRANSFORM, load);
- TableChange updateColumn =
- TableChange.updateColumnNullability(new String[] {col3.name()},
!col3.nullable());
- IllegalArgumentException exception =
- Assertions.assertThrows(
- IllegalArgumentException.class,
- () -> TABLE_OPERATIONS.alterTable(TEST_DB_NAME.toString(),
tableName, updateColumn));
- Assertions.assertTrue(
- exception.getMessage().contains("with null default value cannot be
changed to not null"));
+ // A nullable column whose default is NULL can be changed to not null.
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME.toString(),
+ tableName,
+ TableChange.updateColumnNullability(new String[] {col3.name()},
false));
+ load = TABLE_OPERATIONS.load(TEST_DB_NAME.toString(), tableName);
+ JdbcColumn updatedCol3 =
+ (JdbcColumn)
+ Arrays.stream(load.columns())
+ .filter(c -> c.name().equals(col3.name()))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("Column not found: " +
col3.name()));
+ Assertions.assertFalse(updatedCol3.nullable());
+ Assertions.assertEquals(Column.DEFAULT_VALUE_NOT_SET,
updatedCol3.defaultValue());
}
@Test
diff --git
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
index 9ea122c3e4..21c0f4db2a 100644
---
a/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/main/java/org/apache/gravitino/catalog/postgresql/operation/PostgreSqlTableOperations.java
@@ -55,6 +55,7 @@ import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distribution;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
+import org.apache.gravitino.rel.expressions.literals.Literals;
import org.apache.gravitino.rel.expressions.transforms.Transform;
import org.apache.gravitino.rel.indexes.Index;
import org.apache.gravitino.rel.types.Types;
@@ -369,9 +370,26 @@ public class PostgreSqlTableOperations extends
JdbcTableOperations
(TableChange.UpdateColumnNullability) change;
lazyLoadTable = getOrCreateTable(schemaName, tableName, lazyLoadTable);
- validateUpdateColumnNullable(updateColumnNullability, lazyLoadTable);
+ JdbcColumn column =
validateUpdateColumnNullable(updateColumnNullability, lazyLoadTable);
alterSql.add(updateColumnNullabilityDefinition(updateColumnNullability,
tableName));
+ // A NULL default conflicts with NOT NULL, so drop it when the column
becomes non-nullable,
+ // unless the same request sets a new default value for the column.
+ JdbcColumn pendingColumn =
withPendingDefaultValueAndNullability(column, changes);
+ if (Literals.NULL.equals(column.defaultValue())
+ &&
Column.DEFAULT_VALUE_NOT_SET.equals(pendingColumn.defaultValue())) {
+ alterSql.add(
+ ALTER_TABLE
+ + PG_QUOTE
+ + tableName
+ + PG_QUOTE
+ + " "
+ + ALTER_COLUMN
+ + PG_QUOTE
+ + updateColumnNullability.fieldName()[0]
+ + PG_QUOTE
+ + " DROP DEFAULT;");
+ }
} else if (change instanceof TableChange.AddIndex) {
alterSql.add(addIndexDefinition(tableName, (TableChange.AddIndex)
change));
} else if (change instanceof TableChange.DeleteIndex) {
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
index 13382aa471..6dfa17fe6e 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/operation/TestPostgreSqlTableOperations.java
@@ -22,10 +22,12 @@ import com.google.common.collect.Maps;
import java.sql.Connection;
import java.sql.SQLException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
import javax.sql.DataSource;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.catalog.jdbc.JdbcColumn;
@@ -37,6 +39,7 @@ import
org.apache.gravitino.catalog.postgresql.converter.PostgreSqlColumnDefault
import
org.apache.gravitino.catalog.postgresql.converter.PostgreSqlTypeConverter;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.rel.Column;
import org.apache.gravitino.rel.TableChange;
import org.apache.gravitino.rel.expressions.distributions.Distributions;
import org.apache.gravitino.rel.expressions.literals.Literals;
@@ -238,6 +241,26 @@ public class TestPostgreSqlTableOperations extends
TestPostgreSql {
assertionsTableInfo(
newName, tableComment, alterColumns, properties, null,
Transforms.EMPTY_TRANSFORM, load);
+ // a nullable column with a NULL default can be changed back to not null,
and the NULL
+ // default is dropped
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME,
+ newName,
+ TableChange.updateColumnDefaultValue(new String[] {"col_2"},
Literals.NULL));
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME, newName, TableChange.updateColumnNullability(new
String[] {"col_2"}, false));
+ load = TABLE_OPERATIONS.load(TEST_DB_NAME, newName);
+ alterColumns.set(
+ 1,
+ JdbcColumn.builder()
+ .withName("col_2")
+ .withType(Types.DoubleType.get())
+ .withNullable(false)
+ .withComment("set test key")
+ .build());
+ assertionsTableInfo(
+ newName, tableComment, alterColumns, properties, null,
Transforms.EMPTY_TRANSFORM, load);
+
// delete column
TABLE_OPERATIONS.alterTable(
TEST_DB_NAME,
@@ -269,6 +292,50 @@ public class TestPostgreSqlTableOperations extends
TestPostgreSql {
Assertions.assertFalse(TABLE_OPERATIONS.drop(TEST_DB_NAME, newName));
}
+ @Test
+ public void testUpdateColumnDefaultValueAndNullabilityTogether() {
+ String tableName =
RandomNameUtils.genRandomName("default_not_null_table_");
+ JdbcColumn[] columns = {
+ JdbcColumn.builder()
+ .withName("col_1")
+ .withType(Types.IntegerType.get())
+ .withNullable(true)
+ .withComment("set default first")
+ .build(),
+ JdbcColumn.builder()
+ .withName("col_2")
+ .withType(Types.IntegerType.get())
+ .withNullable(true)
+ .withComment("set not null first")
+ .build()
+ };
+ TABLE_OPERATIONS.create(
+ TEST_DB_NAME,
+ tableName,
+ columns,
+ "test_comment",
+ new HashMap<>(),
+ null,
+ Distributions.NONE,
+ Indexes.EMPTY_INDEXES);
+
+ TABLE_OPERATIONS.alterTable(
+ TEST_DB_NAME,
+ tableName,
+ TableChange.updateColumnDefaultValue(new String[] {"col_1"},
Literals.integerLiteral(5)),
+ TableChange.updateColumnNullability(new String[] {"col_1"}, false),
+ TableChange.updateColumnNullability(new String[] {"col_2"}, false),
+ TableChange.updateColumnDefaultValue(new String[] {"col_2"},
Literals.integerLiteral(7)));
+
+ JdbcTable load = TABLE_OPERATIONS.load(TEST_DB_NAME, tableName);
+ Map<String, Column> loadedColumns =
+ Arrays.stream(load.columns()).collect(Collectors.toMap(Column::name, c
-> c));
+ Assertions.assertFalse(loadedColumns.get("col_1").nullable());
+ Assertions.assertEquals(Literals.integerLiteral(5),
loadedColumns.get("col_1").defaultValue());
+ Assertions.assertFalse(loadedColumns.get("col_2").nullable());
+ Assertions.assertEquals(Literals.integerLiteral(7),
loadedColumns.get("col_2").defaultValue());
+ }
+
@Test
public void testClearCommentsWithNull() {
String tableName = RandomNameUtils.genRandomName("clear_comment_");
