srielau commented on code in PR #59716:
URL: https://github.com/apache/spark/pull/59716#discussion_r4389649017
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala:
##########
@@ -47,7 +48,8 @@ private case class PostgresDialect()
private val supportedAggregateFunctions = Set("MAX", "MIN", "SUM", "COUNT",
"AVG",
"VAR_POP", "VAR_SAMP", "STDDEV_POP", "STDDEV_SAMP", "COVAR_POP",
"COVAR_SAMP", "CORR",
"REGR_INTERCEPT", "REGR_R2", "REGR_SLOPE", "REGR_SXY")
- private val supportedStringFunctions = Set("RPAD", "LPAD")
+ private val supportedStringFunctions =
+ Set("RPAD", "LPAD", CharVarcharUtils.V2_CHAR_READ_SIDE_PADDING)
Review Comment:
Mapped JDBC type name `char` to `StringType` in `getCatalystType` and
`toCatalystType`, so PostgreSQL internal `"char"` is out of read-side padding
and this compile. `JDBCSuite` asserts that mapping; `bpchar(n)` still defers to
`CharType`. Docker coverage is a `"char"` table with empty, `a`, and space. The
4.3-to-4.4 SQL migration guide notes the `CHAR(1)` to `STRING` change.
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala:
##########
@@ -46,7 +47,9 @@ 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", CharVarcharUtils.V2_CHAR_READ_SIDE_PADDING)
Review Comment:
`OracleSQLBuilder.visitIn` now emits OR-ed `IN` chunks of 1000 for longer
lists. `JDBCSuite` compiles a 2-value CHAR IN and NOT IN as before, and a
1001-value `CHAR_READ_SIDE_PADDING` IN as two chunks that contain every literal
once.
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##########
@@ -417,6 +419,16 @@ abstract class JdbcDialect extends Serializable with
Logging {
override def build(expr: Expression): String = expr match {
case _: AlwaysTrue => "(1 = 1)"
case _: AlwaysFalse => "(1 = 0)"
+ case e: GeneralScalarExpression
+ if e.name() == CharVarcharUtils.V2_CHAR_READ_SIDE_PADDING =>
+ if (!isSupportedFunction(CharVarcharUtils.V2_CHAR_READ_SIDE_PADDING)) {
Review Comment:
`JDBCScanBuilder.pushPredicates` now returns predicates that contain
`CHAR_READ_SIDE_PADDING` as unsupported when the dialect does not opt in,
without compiling them. Derby in `JDBCSuite` keeps CHAR equality post-scan,
still pushes unwrapped `IS NULL`, and does not log `Error occurs while
compiling V2 expression`. Opted-in dialects still compile the same SQL.
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/GeneralScalarExpression.java:
##########
@@ -428,6 +428,21 @@
* <li>Since version: 4.0.0</li>
* </ul>
* </li>
+ * <li>Name: <code>CHAR_READ_SIDE_PADDING</code>
+ * <ul>
+ * <li>SQL semantic: <code>CHAR_READ_SIDE_PADDING(str, len, pad)</code>.
+ * <code>len</code> is an integer literal; <code>pad</code> is always a
+ * single space. Right-pads <code>str</code> with spaces to
+ * <code>len</code> characters; values of <code>len</code> or more
+ * characters are returned unchanged. Spark only emits this in
+ * <code>=</code>, <code><=></code>, <code><></code>, and
+ * <code>IN</code> against a string of length <code>len</code>.
+ * A source may compile it as the raw value only if it compares that
Review Comment:
The raw-value permission now requires blank-padded (PAD SPACE) comparison
and stored values that never exceed `len` characters. Padded storage alone is
not enough.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/CharVarcharUtils.scala:
##########
@@ -34,6 +34,22 @@ object CharVarcharUtils extends Logging with
SparkCharVarcharUtils {
// visible for testing
private[sql] val CHAR_VARCHAR_TYPE_STRING_METADATA_KEY =
"__CHAR_VARCHAR_TYPE_STRING"
+ /** JVM method name used by [[addPaddingForScan]] when read-side CHAR
padding is on. */
+ private[sql] val READ_SIDE_PADDING = "readSidePadding"
+
+ /** JVM method name used by [[addPaddingForScan]] under standard
CHAR/VARCHAR semantics. */
+ private[sql] val CHAR_TYPE_READ_SIDE_CHECK = "charTypeReadSideCheck"
+
+ private[sql] val VARCHAR_TYPE_READ_SIDE_CHECK = "varcharTypeReadSideCheck"
+
+ /**
+ * V2 function name for CHAR read-side padding. Distinct from SQL `RPAD` so
connectors that
+ * already allowlist `RPAD` for explicit `rpad()` do not inherit CHAR-scan
rewrite. Emitted
+ * only for = / <=> / <> / IN against a string of the CHAR length. JDBC may
compile it as
+ * the column when CHAR storage is already padded.
Review Comment:
Scaladoc now says JDBC may compile this as the column only when the remote
compares CHAR blank-padded and stored values never exceed that length. It no
longer treats padded storage as sufficient.
--
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]
