-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect) #29972
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.sql.jdbc.v2 | ||
|
|
||
| import java.sql.Connection | ||
|
|
||
| import org.apache.spark.SparkConf | ||
| import org.apache.spark.sql.AnalysisException | ||
| import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog | ||
| import org.apache.spark.sql.jdbc.{DatabaseOnDocker, DockerJDBCIntegrationSuite} | ||
| import org.apache.spark.sql.types._ | ||
| import org.apache.spark.tags.DockerTest | ||
|
|
||
| /** | ||
| * To run this test suite for a specific version (e.g., ibmcom/db2:11.5.4.0): | ||
| * {{{ | ||
| * DB2_DOCKER_IMAGE_NAME=ibmcom/db2:11.5.4.0 | ||
| * ./build/sbt -Pdocker-integration-tests "test-only *DB2IntegrationSuite" | ||
| * }}} | ||
| */ | ||
| @DockerTest | ||
| class DB2IntegrationSuite extends DockerJDBCIntegrationSuite with V2JDBCTest { | ||
| override val catalogName: String = "db2" | ||
| override val db = new DatabaseOnDocker { | ||
| override val imageName = sys.env.getOrElse("DB2_DOCKER_IMAGE_NAME", "ibmcom/db2:11.5.4.0") | ||
| override val env = Map( | ||
| "DB2INST1_PASSWORD" -> "rootpass", | ||
| "LICENSE" -> "accept", | ||
| "DBNAME" -> "foo", | ||
| "ARCHIVE_LOGS" -> "false", | ||
| "AUTOCONFIG" -> "false" | ||
| ) | ||
| override val usesIpc = false | ||
| override val jdbcPort: Int = 50000 | ||
| override val privileged = true | ||
| override def getJdbcUrl(ip: String, port: Int): String = | ||
| s"jdbc:db2://$ip:$port/foo:user=db2inst1;password=rootpass;retrieveMessagesFromServerOnGetMessage=true;" //scalastyle:ignore | ||
| } | ||
|
|
||
| override def sparkConf: SparkConf = super.sparkConf | ||
| .set("spark.sql.catalog.db2", classOf[JDBCTableCatalog].getName) | ||
| .set("spark.sql.catalog.db2.url", db.getJdbcUrl(dockerIp, externalPort)) | ||
|
|
||
| override def dataPreparation(conn: Connection): Unit = {} | ||
|
|
||
cloud-fan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| override def testUpdateColumnType(tbl: String): Unit = { | ||
| sql(s"CREATE TABLE $tbl (ID INTEGER) USING _") | ||
| var t = spark.table(tbl) | ||
| var expectedSchema = new StructType().add("ID", IntegerType) | ||
| assert(t.schema === expectedSchema) | ||
| sql(s"ALTER TABLE $tbl ALTER COLUMN id TYPE DOUBLE") | ||
| t = spark.table(tbl) | ||
| expectedSchema = new StructType().add("ID", DoubleType) | ||
| assert(t.schema === expectedSchema) | ||
| // Update column type from DOUBLE to STRING | ||
| val msg1 = intercept[AnalysisException] { | ||
| sql(s"ALTER TABLE $tbl ALTER COLUMN id TYPE VARCHAR(10)") | ||
| }.getMessage | ||
| assert(msg1.contains("Cannot update alt_table field ID: double cannot be cast to varchar")) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.sql.jdbc.v2 | ||
|
|
||
| import org.apache.spark.sql.AnalysisException | ||
| import org.apache.spark.sql.test.SharedSparkSession | ||
| import org.apache.spark.sql.types._ | ||
| import org.apache.spark.tags.DockerTest | ||
|
|
||
| @DockerTest | ||
| trait V2JDBCTest extends SharedSparkSession { | ||
| val catalogName: String | ||
| // dialect specific update column type test | ||
| def testUpdateColumnType(tbl: String): Unit | ||
|
|
||
| test("SPARK-33034: ALTER TABLE ... add new columns") { | ||
| withTable(s"$catalogName.alt_table") { | ||
| sql(s"CREATE TABLE $catalogName.alt_table (ID STRING) USING _") | ||
| var t = spark.table(s"$catalogName.alt_table") | ||
| var expectedSchema = new StructType().add("ID", StringType) | ||
| assert(t.schema === expectedSchema) | ||
| sql(s"ALTER TABLE $catalogName.alt_table ADD COLUMNS (C1 STRING, C2 STRING)") | ||
| t = spark.table(s"$catalogName.alt_table") | ||
| expectedSchema = expectedSchema.add("C1", StringType).add("C2", StringType) | ||
| assert(t.schema === expectedSchema) | ||
| sql(s"ALTER TABLE $catalogName.alt_table ADD COLUMNS (C3 STRING)") | ||
| t = spark.table(s"$catalogName.alt_table") | ||
| expectedSchema = expectedSchema.add("C3", StringType) | ||
| assert(t.schema === expectedSchema) | ||
| // Add already existing column | ||
| val msg = intercept[AnalysisException] { | ||
| sql(s"ALTER TABLE $catalogName.alt_table ADD COLUMNS (C3 DOUBLE)") | ||
| }.getMessage | ||
| assert(msg.contains("Cannot add column, because C3 already exists")) | ||
| } | ||
| // Add a column to not existing table | ||
| val msg = intercept[AnalysisException] { | ||
| sql(s"ALTER TABLE $catalogName.not_existing_table ADD COLUMNS (C4 STRING)") | ||
| }.getMessage | ||
| assert(msg.contains("Table not found")) | ||
| } | ||
|
|
||
| test("SPARK-33034: ALTER TABLE ... update column type") { | ||
| withTable(s"$catalogName.alt_table") { | ||
| testUpdateColumnType(s"$catalogName.alt_table") | ||
| // Update not existing column | ||
| val msg2 = intercept[AnalysisException] { | ||
| sql(s"ALTER TABLE $catalogName.alt_table ALTER COLUMN bad_column TYPE DOUBLE") | ||
| }.getMessage | ||
| assert(msg2.contains("Cannot update missing field bad_column")) | ||
| } | ||
| // Update column type in not existing table | ||
| val msg = intercept[AnalysisException] { | ||
| sql(s"ALTER TABLE $catalogName.not_existing_table ALTER COLUMN id TYPE DOUBLE") | ||
| }.getMessage | ||
| assert(msg.contains("Table not found")) | ||
| } | ||
|
|
||
| test("SPARK-33034: ALTER TABLE ... update column nullability") { | ||
| withTable(s"$catalogName.alt_table") { | ||
| sql(s"CREATE TABLE $catalogName.alt_table (ID STRING NOT NULL) USING _") | ||
cloud-fan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| var t = spark.table(s"$catalogName.alt_table") | ||
| // nullable is true in the expecteSchema because Spark always sets nullable to true | ||
| // regardless of the JDBC metadata https://github.com/apache/spark/pull/18445 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can change it in JDBC V2, as the table metadata is stored in the remote JDBC server directly. This can be done in a followup.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did a couple of quick tests using V2 write API: and I got Exception from h2 jdbc driver: So we are able to pass the null value for not null column However, if I change the current code in For insert, I got Exception from Spark Spark blocks the insert and we are not able to pass the null value for not null column ID to h2. Since the whole point of #18445 is to let the underlying database to decide how to process null for a not null column, I guess we will not change this
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does expose a problem in Spark: most databases allow to write nullable data to non-nullable column, and fail at runtime if they see null values. I think Spark shouldn't block it at compile time. After all, nullability is more like a constraint, not data type itself. cc @rdblue @dongjoon-hyun @viirya @maropu @MaxGekk |
||
| var expectedSchema = new StructType().add("ID", StringType, nullable = true) | ||
| assert(t.schema === expectedSchema) | ||
| sql(s"ALTER TABLE $catalogName.alt_table ALTER COLUMN ID DROP NOT NULL") | ||
| t = spark.table(s"$catalogName.alt_table") | ||
| expectedSchema = new StructType().add("ID", StringType, nullable = true) | ||
| assert(t.schema === expectedSchema) | ||
| // Update nullability of not existing column | ||
| val msg = intercept[AnalysisException] { | ||
| sql(s"ALTER TABLE $catalogName.alt_table ALTER COLUMN bad_column DROP NOT NULL") | ||
| }.getMessage | ||
| assert(msg.contains("Cannot update missing field bad_column")) | ||
| } | ||
| // Update column nullability in not existing table | ||
| val msg = intercept[AnalysisException] { | ||
| sql(s"ALTER TABLE $catalogName.not_existing_table ALTER COLUMN ID DROP NOT NULL") | ||
| }.getMessage | ||
| assert(msg.contains("Table not found")) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DB2 docker test is much simpler than Oracle? aea78d2#diff-a003dfa2ba6f747fa3ac7f4563e78325R34-R54
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Oracle docker test, it has instructions for how to build docker image. In DB2 docker test and all the other docker tests, it is assumed that the docker images are there and only has instruction for how to run the tests. That's why DB2 docker test looks much simpler.
e.g. here is what we have for MS SQL Server docker test