srielau commented on code in PR #59716:
URL: https://github.com/apache/spark/pull/59716#discussion_r4353059225
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala:
##########
@@ -377,6 +377,17 @@ class V2ExpressionBuilder(e: Expression, isPredicate:
Boolean = false) extends L
case _ =>
None
}
+ // Scan CHAR padding is a JVM invoke; V2 only has RPAD. This matches local
eval when the
+ // stored length is <= n (pad-or-keep then equals truncating RPAD).
Over-long values would
+ // be truncated remotely. charTypeReadSideCheck is left untranslated so
length errors stay
+ // local under spark.sql.charVarchar.standardSemantics.enabled.
+ case StaticInvoke(staticObject, dataType, "readSidePadding", arguments, _,
_, _, _, None)
+ if staticObject == classOf[CharVarcharCodegenUtils] &&
arguments.length == 2 =>
Review Comment:
Follow-up: CHAR_READ_SIDE_PADDING is no longer mapped to RPAD on
H2/Postgres/SQL Server. JDBC compiles it as the column on every dialect that
can push expressions, so we restore `flag = 'Y '` rather than wrapping CHAR
predicates in RPAD. Aggregates that include the padding invoke similarly
compile as the raw column. Explicit rpad() is unchanged.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/V2ExpressionBuilder.scala:
##########
@@ -377,6 +377,17 @@ class V2ExpressionBuilder(e: Expression, isPredicate:
Boolean = false) extends L
case _ =>
None
}
+ // Scan CHAR padding is a JVM invoke; V2 only has RPAD. This matches local
eval when the
+ // stored length is <= n (pad-or-keep then equals truncating RPAD).
Over-long values would
+ // be truncated remotely. charTypeReadSideCheck is left untranslated so
length errors stay
+ // local under spark.sql.charVarchar.standardSemantics.enabled.
+ case StaticInvoke(staticObject, dataType, "readSidePadding", arguments, _,
_, _, _, None)
+ if staticObject == classOf[CharVarcharCodegenUtils] &&
arguments.length == 2 =>
+ generateExpressionWithNameByChildren(
+ "RPAD",
Review Comment:
Follow-up: we reverted the SQL Server CASE wrap around RPAD/LPAD. That was
fixing explicit rpad() NULL handling, which was not part of this regression.
CHAR scan padding now compiles as the column, so `flag = 'Y '` and `flag IS
NULL` never go through CONCAT. The MsSqlServerIntegrationSuite CHAR+NULL cases
still cover IS NULL / NOT IN.
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala:
##########
@@ -46,7 +46,7 @@ private case class OracleDialect() extends JdbcDialect with
SQLConfHelper with N
// scalastyle:on line.size.limit
private val supportedAggregateFunctions =
Set("MAX", "MIN", "SUM", "COUNT", "AVG") ++
distinctUnsupportedAggregateFunctions
- private val supportedFunctions = supportedAggregateFunctions ++ Set("TRUNC")
+ private val supportedFunctions = supportedAggregateFunctions ++ Set("TRUNC",
"RPAD")
Review Comment:
Correction: Oracle was the original regression, so it stays in this PR. We
are not adding RPAD to OracleDialect (your display-width / CJK / empty-string
points still hold for RPAD). CHAR_READ_SIDE_PADDING compiles as the column,
same as other JDBC dialects: well-formed Oracle CHAR is already padded, and
Spark still pads the literal. JDBCSuite expects `("flag") = 'Y '` and still
no SQL RPAD. OracleIntegrationSuite now expects CHAR equality, IS NULL, and CJK
on CHAR(4 CHAR) to be pushed. I have not run the Oracle docker suite here.
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala:
##########
@@ -2350,13 +2350,25 @@ class JDBCSuite extends SharedSparkSession {
assert(getJdbcType(oracleDialect, TimestampNTZType) == "TIMESTAMP")
}
+ test("SPARK-60479: Oracle dialect compiles RPAD for CHAR filter pushdown") {
+ val oracleDialect = JdbcDialects.get("jdbc:oracle://127.0.0.1/db")
+ val rpad = new GeneralScalarExpression("RPAD",
+ Array[V2Expression](
+ FieldReference("PROD_VERSION_CURRENT"),
Review Comment:
Follow-up: the compile test still uses `flag`. It now asserts identity SQL
for H2/Postgres/SQL Server/Oracle/DB2/Derby/MySQL, and Oracle still does not
compile SQL RPAD.
--
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]
