You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by do...@apache.org on 2023/12/27 19:46:55 UTC
(spark) branch branch-3.5 updated: [SPARK-46478][SQL][3.5] Revert SPARK-43049 to use oracle varchar(255) for string
This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch branch-3.5
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.5 by this push:
new 432ab15013b7 [SPARK-46478][SQL][3.5] Revert SPARK-43049 to use oracle varchar(255) for string
432ab15013b7 is described below
commit 432ab15013b7109d020fe66dee1c4287d9bc7cc3
Author: Kent Yao <ya...@apache.org>
AuthorDate: Wed Dec 27 11:46:43 2023 -0800
[SPARK-46478][SQL][3.5] Revert SPARK-43049 to use oracle varchar(255) for string
### What changes were proposed in this pull request?
Revert SPARK-43049 to use Oracle Varchar (255) for string for performance consideration
### Why are the changes needed?
for performance consideration
### Does this PR introduce _any_ user-facing change?
yes, storing strings in Oracle table, which is defined by spark DDL with string columns. Users will get an error if string values exceed 255
```java
org.apache.spark.SparkRuntimeException: [EXCEED_LIMIT_LENGTH] Exceeds char/varchar type length limitation: 255. SQLSTATE: 54006
[info] at org.apache.spark.sql.errors.QueryExecutionErrors$.exceedMaxLimit(QueryExecutionErrors.scala:2512)
```
### How was this patch tested?
revised unit tests
### Was this patch authored or co-authored using generative AI tooling?
no
Closes #44493 from yaooqinn/SPARK-46478-B.
Authored-by: Kent Yao <ya...@apache.org>
Signed-off-by: Dongjoon Hyun <dh...@apple.com>
---
.../spark/sql/jdbc/OracleIntegrationSuite.scala | 3 +--
.../spark/sql/jdbc/v2/OracleIntegrationSuite.scala | 23 ++++++++++++++++------
.../org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala | 2 +-
.../spark/sql/catalyst/util/CharVarcharUtils.scala | 3 ++-
.../org/apache/spark/sql/jdbc/OracleDialect.scala | 2 +-
.../org/apache/spark/sql/jdbc/JDBCSuite.scala | 4 ++--
6 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala b/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
index 483f6087c81d..70afad781ca2 100644
--- a/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
+++ b/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala
@@ -173,8 +173,7 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSpark
}
- // SPARK-43049: Use CLOB instead of VARCHAR(255) for StringType for Oracle jdbc-am""
- test("SPARK-12941: String datatypes to be mapped to CLOB in Oracle") {
+ test("SPARK-12941: String datatypes to be mapped to VARCHAR(255) in Oracle") {
// create a sample dataframe with string type
val df1 = sparkContext.parallelize(Seq(("foo"))).toDF("x")
// write the dataframe to the oracle table tbl
diff --git a/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/OracleIntegrationSuite.scala b/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/OracleIntegrationSuite.scala
index 5124199328ce..6b5dd043a617 100644
--- a/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/OracleIntegrationSuite.scala
+++ b/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/OracleIntegrationSuite.scala
@@ -22,8 +22,9 @@ import java.util.Locale
import org.scalatest.time.SpanSugar._
-import org.apache.spark.SparkConf
+import org.apache.spark.{SparkConf, SparkRuntimeException}
import org.apache.spark.sql.AnalysisException
+import org.apache.spark.sql.catalyst.util.CharVarcharUtils.CHAR_VARCHAR_TYPE_STRING_METADATA_KEY
import org.apache.spark.sql.execution.datasources.v2.jdbc.JDBCTableCatalog
import org.apache.spark.sql.jdbc.DatabaseOnDocker
import org.apache.spark.sql.types._
@@ -86,6 +87,11 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationV2Suite with V2JDBCTes
s"jdbc:oracle:thin:system/$oracle_password@//$ip:$port/xe"
}
+ override val defaultMetadata: Metadata = new MetadataBuilder()
+ .putLong("scale", 0)
+ .putString(CHAR_VARCHAR_TYPE_STRING_METADATA_KEY, "varchar(255)")
+ .build()
+
override def sparkConf: SparkConf = super.sparkConf
.set("spark.sql.catalog.oracle", classOf[JDBCTableCatalog].getName)
.set("spark.sql.catalog.oracle.url", db.getJdbcUrl(dockerIp, externalPort))
@@ -104,11 +110,11 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationV2Suite with V2JDBCTes
override def testUpdateColumnType(tbl: String): Unit = {
sql(s"CREATE TABLE $tbl (ID INTEGER)")
var t = spark.table(tbl)
- var expectedSchema = new StructType().add("ID", DecimalType(10, 0), true, defaultMetadata)
+ var expectedSchema = new StructType().add("ID", DecimalType(10, 0), true, super.defaultMetadata)
assert(t.schema === expectedSchema)
sql(s"ALTER TABLE $tbl ALTER COLUMN id TYPE LONG")
t = spark.table(tbl)
- expectedSchema = new StructType().add("ID", DecimalType(19, 0), true, defaultMetadata)
+ expectedSchema = new StructType().add("ID", DecimalType(19, 0), true, super.defaultMetadata)
assert(t.schema === expectedSchema)
// Update column type from LONG to INTEGER
val sql1 = s"ALTER TABLE $tbl ALTER COLUMN id TYPE INTEGER"
@@ -129,12 +135,17 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationV2Suite with V2JDBCTes
override def caseConvert(tableName: String): String = tableName.toUpperCase(Locale.ROOT)
- test("SPARK-43049: Use CLOB instead of VARCHAR(255) for StringType for Oracle JDBC") {
+ test("SPARK-46478: Revert SPARK-43049 to use varchar(255) for string") {
val tableName = catalogName + ".t1"
withTable(tableName) {
sql(s"CREATE TABLE $tableName(c1 string)")
- sql(s"INSERT INTO $tableName SELECT rpad('hi', 256, 'spark')")
- assert(sql(s"SELECT char_length(c1) from $tableName").head().get(0) === 256)
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ sql(s"INSERT INTO $tableName SELECT rpad('hi', 256, 'spark')")
+ },
+ errorClass = "EXCEED_LIMIT_LENGTH",
+ parameters = Map("limit" -> "255")
+ )
}
}
}
diff --git a/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala b/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
index b5f5b0e5f20b..99f435611f2c 100644
--- a/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
+++ b/connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
@@ -49,7 +49,7 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
def notSupportsTableComment: Boolean = false
- val defaultMetadata = new MetadataBuilder().putLong("scale", 0).build()
+ def defaultMetadata: Metadata = new MetadataBuilder().putLong("scale", 0).build()
def testUpdateColumnNullability(tbl: String): Unit = {
sql(s"CREATE TABLE $catalogName.alt_table (ID STRING NOT NULL)")
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
index b9d83d444909..f3c272785a7b 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala
@@ -28,7 +28,8 @@ import org.apache.spark.sql.types._
object CharVarcharUtils extends Logging with SparkCharVarcharUtils {
- private val CHAR_VARCHAR_TYPE_STRING_METADATA_KEY = "__CHAR_VARCHAR_TYPE_STRING"
+ // visible for testing
+ private[sql] val CHAR_VARCHAR_TYPE_STRING_METADATA_KEY = "__CHAR_VARCHAR_TYPE_STRING"
/**
* Replaces CharType/VarcharType with StringType recursively in the given struct type. If a
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala
index 3a0333cca33f..95774d38e50e 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala
@@ -118,7 +118,7 @@ private case object OracleDialect extends JdbcDialect {
case DoubleType => Some(JdbcType("NUMBER(19, 4)", java.sql.Types.DOUBLE))
case ByteType => Some(JdbcType("NUMBER(3)", java.sql.Types.SMALLINT))
case ShortType => Some(JdbcType("NUMBER(5)", java.sql.Types.SMALLINT))
- case StringType => Some(JdbcType("CLOB", java.sql.Types.CLOB))
+ case StringType => Some(JdbcType("VARCHAR2(255)", java.sql.Types.VARCHAR))
case _ => None
}
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
index 71c7245b0609..c4145f4cbf73 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
@@ -1274,7 +1274,7 @@ class JDBCSuite extends QueryTest with SharedSparkSession {
test("SPARK 12941: The data type mapping for StringType to Oracle") {
val oracleDialect = JdbcDialects.get("jdbc:oracle://127.0.0.1/db")
assert(oracleDialect.getJDBCType(StringType).
- map(_.databaseTypeDefinition).get == "CLOB")
+ map(_.databaseTypeDefinition).get == "VARCHAR2(255)")
}
test("SPARK-16625: General data types to be mapped to Oracle") {
@@ -1292,7 +1292,7 @@ class JDBCSuite extends QueryTest with SharedSparkSession {
assert(getJdbcType(oracleDialect, DoubleType) == "NUMBER(19, 4)")
assert(getJdbcType(oracleDialect, ByteType) == "NUMBER(3)")
assert(getJdbcType(oracleDialect, ShortType) == "NUMBER(5)")
- assert(getJdbcType(oracleDialect, StringType) == "CLOB")
+ assert(getJdbcType(oracleDialect, StringType) == "VARCHAR2(255)")
assert(getJdbcType(oracleDialect, BinaryType) == "BLOB")
assert(getJdbcType(oracleDialect, DateType) == "DATE")
assert(getJdbcType(oracleDialect, TimestampType) == "TIMESTAMP")
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org