srielau commented on code in PR #59716:
URL: https://github.com/apache/spark/pull/59716#discussion_r4361959111


##########
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/OracleIntegrationSuite.scala:
##########
@@ -428,4 +428,36 @@ 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 tableName = caseConvert("char_pad")
+    val table = s"$catalogName.$tableName"
+    withTable(table) {
+      // CHAR(n CHAR) so CJK occupies one length unit, matching Spark CHAR 
padding.
+      Using.resource(getConnection()) { conn =>
+        conn.prepareStatement(
+          s"CREATE TABLE $tableName (id NUMBER, flag CHAR(4 
CHAR))").executeUpdate()
+      }
+      // scalastyle:off nonascii
+      sql(s"INSERT INTO $table VALUES (1, 'Y'), (2, NULL), (3, '日本')")
+      // scalastyle:on nonascii
+      val eq = sql(s"SELECT id FROM $table WHERE flag = 'Y'")
+      checkFilterPushed(eq)
+      val eqRows = eq.collect()
+      assert(eqRows.length === 1)
+      assert(eqRows(0).getInt(0) === 1)

Review Comment:
   Fixed. Bare NUMBER maps to DecimalType(38, 18), so the test now reads ids 
with getDecimal(0).intValueExact().



##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/expressions/GeneralScalarExpression.java:
##########
@@ -428,6 +428,17 @@
  *    <li>Since version: 4.0.0</li>
  *   </ul>
  *  </li>
+ *  <li>Name: <code>CHAR_READ_SIDE_PADDING</code>
+ *   <ul>
+ *    <li>SQL semantic: right-pads <code>str</code> with spaces to 
<code>len</code>

Review Comment:
   Documented the invocation as CHAR_READ_SIDE_PADDING(str, len, pad) with len 
an integer literal and pad a single space. The raw-column compile is now stated 
as blank-padded (PAD SPACE) comparison, not storage.



##########
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2StrategySuite.scala:
##########
@@ -1882,6 +1883,87 @@ 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)
+      .build()
+    assert(nseq.isDefined)
+    val in = new V2ExpressionBuilder(In(padded, Seq(Literal("Y"))), 
isPredicate = true).build()
+    assert(in.isDefined)
+    val neq = new V2ExpressionBuilder(Not(EqualTo(padded, Literal("Y"))), 
isPredicate = true)
+      .build()
+    assert(neq.isDefined)
+    neq.get match {
+      case p: Predicate if p.name() == "<>" =>
+      case other => fail(s"expected '<>' predicate, got ${other.describe()}")
+    }
+    val inSet = new V2ExpressionBuilder(
+      InSet(padded, Set(UTF8String.fromString("Y"))), isPredicate = 
true).build()
+    assert(inSet.isDefined)

Review Comment:
   The InSet unit test now asserts CHAR_READ_SIDE_PADDING plus the padded 
literal. JDBCV2Suite also runs CHAR IN with inSetConversionThreshold=2 so 
OptimizeIn takes the InSet path.



-- 
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]

Reply via email to