Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
* 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.execution.datasources.v2.jdbc.JDBCTableCatalog
import org.apache.spark.sql.jdbc.{DatabaseOnDocker, DockerJDBCIntegrationSuite}
import org.apache.spark.sql.types._
import org.apache.spark.tags.DockerTest

/**
*
* To run this test suite for a specific version (e.g., mysql:5.7.31):
* {{{
* MYSQL_DOCKER_IMAGE_NAME=mysql:5.7.31
* ./build/sbt -Pdocker-integration-tests "testOnly *v2*MySQLIntegrationSuite"
*
* }}}
*
*/
@DockerTest
class MySQLIntegrationSuite extends DockerJDBCIntegrationSuite with V2JDBCTest {
override val catalogName: String = "mysql"
override val db = new DatabaseOnDocker {
override val imageName = sys.env.getOrElse("MYSQL_DOCKER_IMAGE_NAME", "mysql:5.7.31")
override val env = Map(
"MYSQL_ROOT_PASSWORD" -> "rootpass"
)
override val usesIpc = false
override val jdbcPort: Int = 3306

override def getJdbcUrl(ip: String, port: Int): String =
s"jdbc:mysql://$ip:$port/mysql?user=root&password=rootpass"
}

override def sparkConf: SparkConf = super.sparkConf
.set("spark.sql.catalog.mysql", classOf[JDBCTableCatalog].getName)
.set("spark.sql.catalog.mysql.url", db.getJdbcUrl(dockerIp, externalPort))

override val connectionTimeout = timeout(7.minutes)

override def dataPreparation(conn: Connection): Unit = {}

override def testUpdateColumnType(tbl: String): Unit = {
sql(s"CREATE TABLE $tbl (ID INTEGER) USING _")
var t = spark.table(tbl)
var expectedSchema = new StructType().add("ID", IntegerType)
assert(t.schema === expectedSchema)
sql(s"ALTER TABLE $tbl ALTER COLUMN id TYPE STRING")
t = spark.table(tbl)
expectedSchema = new StructType().add("ID", StringType)
assert(t.schema === expectedSchema)
// Update column type from STRING to INTEGER
val msg1 = intercept[AnalysisException] {
sql(s"ALTER TABLE $tbl ALTER COLUMN id TYPE INTEGER")
}.getMessage
assert(msg1.contains("Cannot update alt_table field ID: string cannot be cast to int"))
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -893,10 +893,11 @@ object JdbcUtils extends Logging {
conn: Connection,
tableName: String,
changes: Seq[TableChange],
options: JDBCOptions): Unit = {
options: JDBCOptions,
tableSchema: StructType): Unit = {
val dialect = JdbcDialects.get(options.url)
if (changes.length == 1) {
executeStatement(conn, options, dialect.alterTable(tableName, changes)(0))
executeStatement(conn, options, dialect.alterTable(tableName, changes, tableSchema)(0))
} else {
val metadata = conn.getMetaData
if (!metadata.supportsTransactions) {
Expand All @@ -907,7 +908,7 @@ object JdbcUtils extends Logging {
val statement = conn.createStatement
try {
statement.setQueryTimeout(options.queryTimeout)
for (sql <- dialect.alterTable(tableName, changes)) {
for (sql <- dialect.alterTable(tableName, changes, tableSchema)) {
statement.executeUpdate(sql)
}
conn.commit()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ class JDBCTableCatalog extends TableCatalog with Logging {
checkNamespace(ident.namespace())
withConnection { conn =>
classifyException(s"Failed table altering: $ident") {
JdbcUtils.alterTable(conn, getTableName(ident), changes, options)
val optionsWithTableName = new JDBCOptions(
options.parameters + (JDBCOptions.JDBC_TABLE_NAME -> getTableName(ident)))
val tableSchema: StructType = JdbcUtils.getSchemaOption(conn, optionsWithTableName).get

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah now I get what you mean. The AlterTable logical plan does have the table schema, but the catalog API doesn't pass it in. It's not possible to change the catalog API at this point, and it's also not worthy to add an extra table lookup here just to support update nullability in MySQL.

I think the first version is fine. Sorry for the back and forth!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks will do so ! 👍

JdbcUtils.alterTable(conn, getTableName(ident), changes, options, tableSchema)
}
loadTable(ident)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ private object DB2Dialect extends JdbcDialect {
override def getUpdateColumnNullabilityQuery(
tableName: String,
columnName: String,
isNullable: Boolean): String = {
isNullable: Boolean,
dataType: String): String = {
val nullable = if (isNullable) "DROP NOT NULL" else "SET NOT NULL"
s"ALTER TABLE $tableName ALTER COLUMN ${quoteIdentifier(columnName)} $nullable"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ import org.apache.commons.lang3.StringUtils

import org.apache.spark.annotation.{DeveloperApi, Since}
import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.analysis.{caseInsensitiveResolution, caseSensitiveResolution}
import org.apache.spark.sql.connector.catalog.TableChange
import org.apache.spark.sql.connector.catalog.TableChange._
import org.apache.spark.sql.execution.datasources.jdbc.JdbcUtils
import org.apache.spark.sql.internal.SQLConf
import org.apache.spark.sql.types._

/**
Expand Down Expand Up @@ -200,12 +202,17 @@ abstract class JdbcDialect extends Serializable {

/**
* Alter an existing table.
* TODO (SPARK-32523): Override this method in the dialects that have different syntax.
*
* @param tableName The name of the table to be altered.
* @param changes Changes to apply to the table.
* @param tableSchema Schema of the table to be altered.
* @return The SQL statements to use for altering the table.
*/
def alterTable(tableName: String, changes: Seq[TableChange]): Array[String] = {
def alterTable(
tableName: String,
changes: Seq[TableChange],
tableSchema: StructType): Array[String] = {
val updateClause = ArrayBuilder.make[String]
for (change <- changes) {
change match {
Expand All @@ -226,7 +233,16 @@ abstract class JdbcDialect extends Serializable {
updateClause += getUpdateColumnTypeQuery(tableName, name(0), dataType)
case updateNull: UpdateColumnNullability if updateNull.fieldNames.length == 1 =>
val name = updateNull.fieldNames
updateClause += getUpdateColumnNullabilityQuery(tableName, name(0), updateNull.nullable())
val columnNameEquality = if (SQLConf.get.caseSensitiveAnalysis) {
caseSensitiveResolution
} else {
caseInsensitiveResolution
}
val columnDataType =
tableSchema.filter(x => columnNameEquality(x.name, name(0))).head.dataType
val jdbcDataType = JdbcUtils.getJdbcType(columnDataType, this).databaseTypeDefinition
updateClause +=
getUpdateColumnNullabilityQuery(tableName, name(0), updateNull.nullable(), jdbcDataType)
case _ =>
throw new SQLFeatureNotSupportedException(s"Unsupported TableChange $change")
}
Expand All @@ -253,7 +269,8 @@ abstract class JdbcDialect extends Serializable {
def getUpdateColumnNullabilityQuery(
tableName: String,
columnName: String,
isNullable: Boolean): String = {
isNullable: Boolean,
dataType: String): String = {
val nullable = if (isNullable) "NULL" else "NOT NULL"
s"ALTER TABLE $tableName ALTER COLUMN ${quoteIdentifier(columnName)} SET $nullable"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,21 @@ private case object MySQLDialect extends JdbcDialect {
}

override def isCascadingTruncateTable(): Option[Boolean] = Some(false)

// See https://dev.mysql.com/doc/refman/8.0/en/alter-table.html
override def getUpdateColumnTypeQuery(
tableName: String,
columnName: String,
newDataType: String): String = {
s"ALTER TABLE $tableName MODIFY COLUMN ${quoteIdentifier(columnName)} $newDataType"
}

override def getUpdateColumnNullabilityQuery(
tableName: String,
columnName: String,
isNullable: Boolean,
dataType: String): String = {
val nullable = if (isNullable) "NULL" else "NOT NULL"
s"ALTER TABLE $tableName MODIFY COLUMN ${quoteIdentifier(columnName)} $dataType $nullable"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ private case object OracleDialect extends JdbcDialect {
override def getUpdateColumnNullabilityQuery(
tableName: String,
columnName: String,
isNullable: Boolean): String = {
isNullable: Boolean,
dataType: String): String = {
val nullable = if (isNullable) "NULL" else "NOT NULL"
s"ALTER TABLE $tableName MODIFY ${quoteIdentifier(columnName)} $nullable"
}
Expand Down