srielau commented on code in PR #59716:
URL: https://github.com/apache/spark/pull/59716#discussion_r4354954753
##########
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/OracleIntegrationSuite.scala:
##########
@@ -428,4 +428,35 @@ class OracleIntegrationSuite extends
DockerJDBCIntegrationV2Suite with V2JDBCTes
assert(rows11(0).getString(0) === "amy")
assert(rows11(1).getString(0) === "alex")
}
+
+ test("SPARK-60479: CHAR equality is pushed, including CJK") {
+ val table = s"$catalogName.char_pad"
Review Comment:
The Spark table name now uses caseConvert so it matches Oracle CHAR_PAD. The
JDBC CREATE TABLE uses the same identifier.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/GeneralScalarExpression.java:
##########
@@ -428,6 +428,16 @@
* <li>Since version: 4.0.0</li>
* </ul>
* </li>
+ * <li>Name: <code>CHAR_READ_SIDE_PADDING</code>
+ * <ul>
+ * <li>SQL semantic: <code>RPAD(str, len, ' ')</code> on a CHAR scan column
when
+ * read-side padding is on. Spark only emits this in <code>=</code>,
+ * <code><=></code>, <code><></code>, and <code>IN</code>
against
+ * a string of length <code>len</code>. JDBC sources that store CHAR
already
+ * padded may compile it as the raw column.</li>
Review Comment:
Updated. The javadoc now describes pad-or-keep: values of len or more
characters are returned unchanged.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala:
##########
@@ -458,6 +478,91 @@ class V2ExpressionBuilder(e: Expression, isPredicate:
Boolean = false) extends L
}
}
+ private def isCharReadSidePaddingInvoke(invoke: StaticInvoke): Boolean = {
+ invoke.scalarFunction.isEmpty &&
+ invoke.staticObject == classOf[CharVarcharCodegenUtils] &&
+ invoke.functionName == CharVarcharUtils.READ_SIDE_PADDING &&
+ invoke.arguments.length == 2
+ }
+
+ // Identity compile is only valid vs a literal Spark already padded to the
CHAR length.
+ private def charPadLength(invoke: StaticInvoke): Option[Int] = {
+ val len = invoke.arguments(1)
+ if (len.foldable) {
+ len.eval() match {
+ case n: Int => Some(n)
+ case _ => None
+ }
+ } else {
+ None
+ }
+ }
+
+ private def isFoldableStringOfLen(expr: Expression, n: Int): Boolean = {
+ expr.foldable && (expr.eval() match {
Review Comment:
Both helpers now match Literal only, so a foldable expression that
ConstantFolding left unevaluated is not eval()'d during planning.
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##########
@@ -467,7 +479,12 @@ abstract class JdbcDialect extends Serializable with
Logging {
}
override def visitSQLFunction(funcName: String, inputs: Array[String]):
String = {
- s"""${dialectFunctionName(funcName)}(${inputs.mkString(", ")})"""
+ funcName match {
+ // JDBC CHAR is stored padded. readSidePadding wrapped the column and
blocked
+ // pushdown; compiling it as the column restores col = 'Y '
(literal padded).
+ case CharVarcharUtils.V2_CHAR_READ_SIDE_PADDING => inputs(0)
Review Comment:
Dropped. CHAR_READ_SIDE_PADDING is compiled in JDBCSQLBuilder.build and no
longer has a visitSQLFunction case.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2StrategySuite.scala:
##########
@@ -1882,6 +1883,71 @@ class DataSourceV2StrategySuite extends
SharedSparkSession {
}
}
+ test("SPARK-60479: read-side CHAR padding StaticInvoke translates") {
+ val md = new MetadataBuilder()
+ .putString(CharVarcharUtils.CHAR_VARCHAR_TYPE_STRING_METADATA_KEY,
"char(1)")
+ .build()
+ val col = AttributeReference("flag", StringType, metadata = md)()
+ val padded = CharVarcharUtils.addPaddingForScan(col)
+ val eq = EqualTo(padded, Literal("Y"))
+ val result = new V2ExpressionBuilder(eq, isPredicate = true).build()
+ assert(result.isDefined, "readSidePadding should translate to a V2
predicate")
+ result.get match {
+ case p: Predicate if p.name() == "=" =>
+ p.children()(0) match {
+ case g: GeneralScalarExpression
+ if g.name() == CharVarcharUtils.V2_CHAR_READ_SIDE_PADDING =>
+ assert(g.children()(0) === FieldReference("flag"))
+ assert(g.children()(1) === LiteralValue(1, IntegerType))
+ assert(g.children()(2) ===
+ LiteralValue(UTF8String.fromString(" "), StringType))
+ case other =>
+ fail(s"expected CHAR_READ_SIDE_PADDING, got ${other.describe()}")
+ }
+ assert(p.children()(1) === LiteralValue(UTF8String.fromString("Y"),
StringType))
+ case other =>
+ fail(s"expected '=' predicate, got ${other.describe()}")
+ }
+
+ val isNull = new V2ExpressionBuilder(IsNull(padded), isPredicate =
true).build()
+ assert(isNull.isDefined)
+ isNull.get match {
+ case p: Predicate if p.name() == "IS_NULL" =>
+ assert(p.children()(0) === FieldReference("flag"))
+ case other => fail(s"expected IS_NULL on the raw column, got
${other.describe()}")
+ }
+
+ val nseq = new V2ExpressionBuilder(EqualNullSafe(padded, Literal("Y")),
isPredicate = true)
Review Comment:
Added. Not(EqualTo) becomes <>, InSet translates, IN with a NULL literal
stays local, and EqualTo(Literal, padded) still translates.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
