You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/10/07 23:06:32 UTC
[GitHub] [spark] huaxingao opened a new pull request #29972: Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
huaxingao opened a new pull request #29972:
URL: https://github.com/apache/spark/pull/29972
### What changes were proposed in this pull request?
- Override the default SQL strings in the DB2 Dialect for:
* ALTER TABLE UPDATE COLUMN TYPE
* ALTER TABLE UPDATE COLUMN NULLABILITY
- Add new docker integration test suite jdbc/v2/DB2IntegrationSuite.scala
### Why are the changes needed?
In SPARK-24907, we implemented JDBC v2 Table Catalog but it doesn't support some ALTER TABLE at the moment. This PR supports DB2 specific ALTER TABLE.
### Does this PR introduce _any_ user-facing change?
Yes
### How was this patch tested?
By running new integration test suite:
$ ./build/sbt -Pdocker-integration-tests "test-only *.DB2IntegrationSuite"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707914377
Thanks! @cloud-fan @MaxGekk
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #29972:
URL: https://github.com/apache/spark/pull/29972
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707419405
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705915047
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705915047
**[Test build #129569 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129569/testReport)** for PR 29972 at commit [`6c2919d`](https://github.com/apache/spark/commit/6c2919df7f41815c9af246ba40ae353928636e0d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705279064
**[Test build #129534 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129534/testReport)** for PR 29972 at commit [`8e2da8d`](https://github.com/apache/spark/commit/8e2da8d38f5af63ae284f1d8823cb1e75ef61493).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705916901
**[Test build #129570 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129570/testReport)** for PR 29972 at commit [`0f9284b`](https://github.com/apache/spark/commit/0f9284b5a154789e107a0aebd13ce99c2d1a9d2c).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk edited a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
MaxGekk edited a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705382761
> For example, DB2 doesn't have a STRING type. It uses CHAR and VARCHAR.
You could define a type in the common trait like `StrType`, and override in the Oracle and DB2 test suites.
> Oracle allows update column data type from INTEGER to STRING, but DB2 doesn't allow update column data type from INTEGER to VARCHAR.
ohh, I think we can remove such checks because:
- I don't see any reasons to test DBMS behavior in Spark's tests
- The checks doesn't improve test coverage.
or as an option, we could extract dialect specific test to separate tests. For now, while reading the integration tests, it is hard to say why we duplicate the code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705932263
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705928191
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34176/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705932254
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34175/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r501447583
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.scalatest.time.SpanSugar._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.parser.ParseException
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.jdbc.{DatabaseOnDocker, DockerJDBCIntegrationSuite}
+import org.apache.spark.sql.test.SharedSparkSession
+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 SharedSparkSession {
+ 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 = {}
+
Review comment:
can we test `ADD COLUMN` as well? If it's not supported, we can check the error message in test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r501447286
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.scalatest.time.SpanSugar._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.parser.ParseException
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.jdbc.{DatabaseOnDocker, DockerJDBCIntegrationSuite}
+import org.apache.spark.sql.test.SharedSparkSession
+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
Review comment:
DB2 docker test is much simpler than Oracle? https://github.com/apache/spark/commit/aea78d2c8cdf12f4978fa6a69107d096c07c6fec#diff-a003dfa2ba6f747fa3ac7f4563e78325R34-R54
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705932263
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705934006
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705240773
**[Test build #129534 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129534/testReport)** for PR 29972 at commit [`8e2da8d`](https://github.com/apache/spark/commit/8e2da8d38f5af63ae284f1d8823cb1e75ef61493).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705279590
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705915047
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r504154239
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -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 _")
+ 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
Review comment:
I did a couple of quick tests using V2 write API:
```
sql("INSERT INTO h2.test.people SELECT 'bob', null")
```
and
```
sql("SELECT null AS ID, 'bob' AS NAME").writeTo("h2.test.people")
```
I got Exception from h2 jdbc driver:
```
Caused by: org.h2.jdbc.JdbcSQLException: NULL not allowed for column "ID"; SQL statement:
INSERT INTO "test"."people" ("NAME","ID") VALUES (?,?) [23502-195]
at org.h2.message.DbException.getJdbcSQLException(DbException.java:345)
```
So we are able to pass the null value for not null column `ID` to h2 and h2 blocks the insert.
However, if I change the current code in `JDBCRDD.resolveTable` to make `alwaysNullable = false` to get the real nullable value,
```
def resolveTable(options: JDBCOptions): StructType = {
......
JdbcUtils.getSchema(rs, dialect, alwaysNullable = false)
```
For insert, I got Exception from Spark
```
Cannot write incompatible data to table 'test.people':
- Cannot write nullable values to non-null column 'ID';
org.apache.spark.sql.AnalysisException: Cannot write incompatible data to table 'test.people':
- Cannot write nullable values to non-null column 'ID';
at org.apache.spark.sql.catalyst.analysis.TableOutputResolver$.resolveOutputColumns(TableOutputResolver.scala:72)
at org.apache.spark.sql.catalyst.analysis.Analyzer$ResolveOutputRelation$$anonfun$apply$31.applyOrElse(Analyzer.scala:3040)
at org.apache.spark.sql.catalyst.analysis.Analyzer$ResolveOutputRelation$$anonfun$apply$31.applyOrElse(Analyzer.scala:3035)
```
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 https://github.com/apache/spark/pull/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 `alwaysNullable` for JDBCV2?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705372646
> Could you put the tests to a common trait, and mix it to Oracle and DB2 dialect test suites. The diff should be only in catalog name oracle vs db2, right?
There are actually a little more differences. For example, DB2 doesn't have a STRING type. It uses CHAR and VARCHAR. Also, Oracle allows update column data type from INTEGER to STRING, but DB2 doesn't allow update column data type from INTEGER to VARCHAR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705259491
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r503149449
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.{DoubleType, StructType}
+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 = {}
+
+ override def updateColumnType: Unit = {
+ sql(s"CREATE TABLE $catalogName.alt_table (ID INTEGER) USING _")
Review comment:
can we test the table schema right after creation?
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/OracleIntegrationSuite.scala
##########
@@ -73,80 +72,16 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSpark
override val connectionTimeout = timeout(7.minutes)
override def dataPreparation(conn: Connection): Unit = {}
- test("SPARK-33034: ALTER TABLE ... add new columns") {
- withTable("oracle.alt_table") {
- sql("CREATE TABLE oracle.alt_table (ID STRING) USING _")
- sql("ALTER TABLE oracle.alt_table ADD COLUMNS (C1 STRING, C2 STRING)")
- var t = spark.table("oracle.alt_table")
- var expectedSchema = new StructType()
- .add("ID", StringType)
- .add("C1", StringType)
- .add("C2", StringType)
- assert(t.schema === expectedSchema)
- sql("ALTER TABLE oracle.alt_table ADD COLUMNS (C3 STRING)")
- t = spark.table("oracle.alt_table")
- expectedSchema = expectedSchema.add("C3", StringType)
- assert(t.schema === expectedSchema)
- // Add already existing column
- val msg = intercept[AnalysisException] {
- sql(s"ALTER TABLE oracle.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 oracle.not_existing_table ADD COLUMNS (C4 STRING)")
+ override def updateColumnType: Unit = {
+ sql(s"CREATE TABLE $catalogName.alt_table (ID INTEGER) USING _")
Review comment:
ditto
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705932263
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r501482051
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.scalatest.time.SpanSugar._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.parser.ParseException
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.jdbc.{DatabaseOnDocker, DockerJDBCIntegrationSuite}
+import org.apache.spark.sql.test.SharedSparkSession
+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
Review comment:
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
```
/**
* To run this test suite for a specific version (e.g., 2019-GA-ubuntu-16.04):
* {{{
* MSSQLSERVER_DOCKER_IMAGE_NAME=2019-GA-ubuntu-16.04
* ./build/sbt -Pdocker-integration-tests "test-only *MsSqlServerIntegrationSuite"
* }}}
*/
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707370035
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34312/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705934012
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34176/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707351433
**[Test build #129706 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129706/testReport)** for PR 29972 at commit [`8d0226b`](https://github.com/apache/spark/commit/8d0226b9e8fcee978c2d601054084a4c357692d5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705958525
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707418695
**[Test build #129706 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129706/testReport)** for PR 29972 at commit [`8d0226b`](https://github.com/apache/spark/commit/8d0226b9e8fcee978c2d601054084a4c357692d5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707376053
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34312/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705372646
> Could you put the tests to a common trait, and mix it to Oracle and DB2 dialect test suites. The diff should be only in catalog name oracle vs db2, right?
There are actually a little more differences. For example, DB2 doesn't have a STRING type. It uses CHAR and VARCHAR. Also, Oracle allows update column data type from INTEGER to STRING, but DB2 doesn't allow update column data type from INTEGER to VARCHAR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705927763
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34175/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705279590
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r503149079
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.catalyst.parser.ParseException
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.sql.types.{StringType, StructType}
+import org.apache.spark.tags.DockerTest
+
+@DockerTest
+trait V2JDBCTest extends SharedSparkSession {
+ val catalogName: String
+ // dialect specific update column type test
+ def updateColumnType: Unit
+
+ test("SPARK-33034: ALTER TABLE ... add new columns") {
+ withTable(s"$catalogName.alt_table") {
+ sql(s"CREATE TABLE $catalogName.alt_table (ID STRING) USING _")
+ sql(s"ALTER TABLE $catalogName.alt_table ADD COLUMNS (C1 STRING, C2 STRING)")
+ var t = spark.table(s"$catalogName.alt_table")
+ var expectedSchema = new StructType()
+ .add("ID", StringType)
+ .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") {
+ updateColumnType
Review comment:
ah it has tests as well. How about `testUpdateColumnType(tbl: String)`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705259496
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34139/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705958079
**[Test build #129570 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129570/testReport)** for PR 29972 at commit [`0f9284b`](https://github.com/apache/spark/commit/0f9284b5a154789e107a0aebd13ce99c2d1a9d2c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `trait V2JDBCTest extends SharedSparkSession `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r503927615
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -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 _")
+ 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
Review comment:
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r502125761
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.catalyst.parser.ParseException
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.sql.types.{StringType, StructType}
+import org.apache.spark.tags.DockerTest
+
+@DockerTest
+trait V2JDBCTest extends SharedSparkSession {
+ val catalogName: String
+ // dialect specific update column type test
+ def updateColumnType: Unit
Review comment:
I can't find an update column type that works for both DB2 and Oracle, so I will do this separately. For DB2, we have update column type from int to double. For Oracle, we have update column type from int to string
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705934000
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34176/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r503147935
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.catalyst.parser.ParseException
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.sql.types.{StringType, StructType}
+import org.apache.spark.tags.DockerTest
+
+@DockerTest
+trait V2JDBCTest extends SharedSparkSession {
+ val catalogName: String
+ // dialect specific update column type test
+ def updateColumnType: Unit
+
+ test("SPARK-33034: ALTER TABLE ... add new columns") {
+ withTable(s"$catalogName.alt_table") {
+ sql(s"CREATE TABLE $catalogName.alt_table (ID STRING) USING _")
+ sql(s"ALTER TABLE $catalogName.alt_table ADD COLUMNS (C1 STRING, C2 STRING)")
+ var t = spark.table(s"$catalogName.alt_table")
+ var expectedSchema = new StructType()
+ .add("ID", StringType)
+ .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") {
+ updateColumnType
+ // 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 to wrong type
+ val msg3 = intercept[ParseException] {
+ sql(s"ALTER TABLE $catalogName.alt_table ALTER COLUMN id TYPE bad_type")
+ }.getMessage
+ assert(msg3.contains("DataType bad_type is not supported"))
+ }
+ // 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 _")
Review comment:
can we test the schema of the table right after creation?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r501482812
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.scalatest.time.SpanSugar._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.parser.ParseException
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.jdbc.{DatabaseOnDocker, DockerJDBCIntegrationSuite}
+import org.apache.spark.sql.test.SharedSparkSession
+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 SharedSparkSession {
+ 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 = {}
+
Review comment:
`ADD COLUMN` is supported by DB2. I was following https://github.com/apache/spark/pull/29912 and only test the ALTER TABLE syntax that are overridden by the specific database dialect, in this case, update column type and nullability. But we probably need to test all of ALTER TABLE syntax: add column, delete column, rename column, update column type and nullability in each of the dock test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705932263
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705253646
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34139/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r503146094
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.catalyst.parser.ParseException
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.sql.types.{StringType, StructType}
+import org.apache.spark.tags.DockerTest
+
+@DockerTest
+trait V2JDBCTest extends SharedSparkSession {
+ val catalogName: String
+ // dialect specific update column type test
+ def updateColumnType: Unit
+
+ test("SPARK-33034: ALTER TABLE ... add new columns") {
+ withTable(s"$catalogName.alt_table") {
+ sql(s"CREATE TABLE $catalogName.alt_table (ID STRING) USING _")
+ sql(s"ALTER TABLE $catalogName.alt_table ADD COLUMNS (C1 STRING, C2 STRING)")
+ var t = spark.table(s"$catalogName.alt_table")
+ var expectedSchema = new StructType()
+ .add("ID", StringType)
+ .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") {
+ updateColumnType
+ // 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 to wrong type
+ val msg3 = intercept[ParseException] {
+ sql(s"ALTER TABLE $catalogName.alt_table ALTER COLUMN id TYPE bad_type")
Review comment:
I think we should remove it as it's a parser error which is not related to JDBC.
We can move it to `DDLParserSuite` if it's not tested there.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705259481
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34139/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r503145481
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.catalyst.parser.ParseException
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.sql.types.{StringType, StructType}
+import org.apache.spark.tags.DockerTest
+
+@DockerTest
+trait V2JDBCTest extends SharedSparkSession {
+ val catalogName: String
+ // dialect specific update column type test
+ def updateColumnType: Unit
+
+ test("SPARK-33034: ALTER TABLE ... add new columns") {
+ withTable(s"$catalogName.alt_table") {
+ sql(s"CREATE TABLE $catalogName.alt_table (ID STRING) USING _")
+ sql(s"ALTER TABLE $catalogName.alt_table ADD COLUMNS (C1 STRING, C2 STRING)")
+ var t = spark.table(s"$catalogName.alt_table")
+ var expectedSchema = new StructType()
+ .add("ID", StringType)
+ .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") {
+ updateColumnType
Review comment:
should the name be `prepareTableForUpdateTypeTest(tbl: String)`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705382761
> For example, DB2 doesn't have a STRING type. It uses CHAR and VARCHAR.
You could define a type in the common trait like `StrType`, and override in the Oracle and DB2 test suites.
> Oracle allows update column data type from INTEGER to STRING, but DB2 doesn't allow update column data type from INTEGER to VARCHAR.
ohh, I think we can remove such checks because:
- I don't see any reasons to test DBMS behavior in Spark's tests
- The checks doesn't improve test coverage.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705240773
**[Test build #129534 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129534/testReport)** for PR 29972 at commit [`8e2da8d`](https://github.com/apache/spark/commit/8e2da8d38f5af63ae284f1d8823cb1e75ef61493).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r504374789
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -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 _")
+ 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
Review comment:
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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r501447286
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.scalatest.time.SpanSugar._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.parser.ParseException
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.jdbc.{DatabaseOnDocker, DockerJDBCIntegrationSuite}
+import org.apache.spark.sql.test.SharedSparkSession
+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
Review comment:
DB2 docker test is much simpler than Oracle? https://github.com/apache/spark/commit/aea78d2c8cdf12f4978fa6a69107d096c07c6fec#diff-a003dfa2ba6f747fa3ac7f4563e78325R34-R54
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.scalatest.time.SpanSugar._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.parser.ParseException
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.jdbc.{DatabaseOnDocker, DockerJDBCIntegrationSuite}
+import org.apache.spark.sql.test.SharedSparkSession
+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 SharedSparkSession {
+ 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 = {}
+
Review comment:
can we test `ADD COLUMN` as well? If it's not supported, we can check the error message in test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705259491
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707376068
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707376068
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705956794
**[Test build #129569 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129569/testReport)** for PR 29972 at commit [`6c2919d`](https://github.com/apache/spark/commit/6c2919df7f41815c9af246ba40ae353928636e0d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `class DB2IntegrationSuite extends DockerJDBCIntegrationSuite with V2JDBCTest `
* `class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with V2JDBCTest `
* `trait V2JDBCTest extends SharedSparkSession `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705915047
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk edited a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
MaxGekk edited a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705382761
> For example, DB2 doesn't have a STRING type. It uses CHAR and VARCHAR.
You could define a type in the common trait like `StrType`, and override in the Oracle and DB2 test suites.
> Oracle allows update column data type from INTEGER to STRING, but DB2 doesn't allow update column data type from INTEGER to VARCHAR.
ohh, I think we can remove such checks because:
- I don't see any reasons to test DBMS behavior in Spark's tests
- The checks doesn't improve test coverage.
or as an option, we could extract dialect specific test to separate tests. For now, while reading the integration tests, it is hard to say why we duplicate the code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705957305
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705957305
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on a change in pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on a change in pull request #29972:
URL: https://github.com/apache/spark/pull/29972#discussion_r501482051
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.scalatest.time.SpanSugar._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.parser.ParseException
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.jdbc.{DatabaseOnDocker, DockerJDBCIntegrationSuite}
+import org.apache.spark.sql.test.SharedSparkSession
+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
Review comment:
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
```
/**
* To run this test suite for a specific version (e.g., 2019-GA-ubuntu-16.04):
* {{{
* MSSQLSERVER_DOCKER_IMAGE_NAME=2019-GA-ubuntu-16.04
* ./build/sbt -Pdocker-integration-tests "test-only *MsSqlServerIntegrationSuite"
* }}}
*/
```
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/DB2IntegrationSuite.scala
##########
@@ -0,0 +1,113 @@
+/*
+ * 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.scalatest.time.SpanSugar._
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.parser.ParseException
+import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
+import org.apache.spark.sql.jdbc.{DatabaseOnDocker, DockerJDBCIntegrationSuite}
+import org.apache.spark.sql.test.SharedSparkSession
+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 SharedSparkSession {
+ 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 = {}
+
Review comment:
`ADD COLUMN` is supported by DB2. I was following https://github.com/apache/spark/pull/29912 and only test the ALTER TABLE syntax that are overridden by the specific database dialect, in this case, update column type and nullability. But we probably need to test all of ALTER TABLE syntax: add column, delete column, rename column, update column type and nullability in each of the dock test.
##########
File path: external/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
##########
@@ -0,0 +1,99 @@
+/*
+ * 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.catalyst.parser.ParseException
+import org.apache.spark.sql.test.SharedSparkSession
+import org.apache.spark.sql.types.{StringType, StructType}
+import org.apache.spark.tags.DockerTest
+
+@DockerTest
+trait V2JDBCTest extends SharedSparkSession {
+ val catalogName: String
+ // dialect specific update column type test
+ def updateColumnType: Unit
Review comment:
I can't find an update column type that works for both DB2 and Oracle, so I will do this separately. For DB2, we have update column type from int to double. For Oracle, we have update column type from int to string
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] huaxingao commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
huaxingao commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705292088
cc @cloud-fan @maropu @MaxGekk Could you please take a look? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707351433
**[Test build #129706 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129706/testReport)** for PR 29972 at commit [`8d0226b`](https://github.com/apache/spark/commit/8d0226b9e8fcee978c2d601054084a4c357692d5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] MaxGekk commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-705382761
> For example, DB2 doesn't have a STRING type. It uses CHAR and VARCHAR.
You could define a type in the common trait like `StrType`, and override in the Oracle and DB2 test suites.
> Oracle allows update column data type from INTEGER to STRING, but DB2 doesn't allow update column data type from INTEGER to VARCHAR.
ohh, I think we can remove such checks because:
- I don't see any reasons to test DBMS behavior in Spark's tests
- The checks doesn't improve test coverage.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707720313
thanks, merging to master!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #29972: [SPARK-33081][SQL] Support ALTER TABLE in JDBC v2 Table Catalog: update type and nullability of columns (DB2 dialect)
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #29972:
URL: https://github.com/apache/spark/pull/29972#issuecomment-707419405
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org