You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "zhengruifeng (via GitHub)" <gi...@apache.org> on 2023/06/19 08:59:43 UTC

[GitHub] [spark] zhengruifeng opened a new pull request, #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

zhengruifeng opened a new pull request, #41659:
URL: https://github.com/apache/spark/pull/41659

   ### What changes were proposed in this pull request?
   Make `startswith`, `endswith` support binary type:
   1, in Connect, `startswith` & `endswith` actually already support binary type;
   2, in vanilla Scala API, support binary type via `call_udf`
   
   
   ### Why are the changes needed?
   for parity
   
   
   ### Does this PR introduce _any_ user-facing change?
   yes
   
   
   ### How was this patch tested?
   added ut


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] amaliujia commented on pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #41659:
URL: https://github.com/apache/spark/pull/41659#issuecomment-1598060101

   is there a need to also update the example at https://github.com/apache/spark/blob/master/sql/core/src/test/resources/sql-functions/sql-expression-schema.md to indicate binary type is supported?


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zhengruifeng commented on pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #41659:
URL: https://github.com/apache/spark/pull/41659#issuecomment-1597899468

   > 
   
   I don't think so, since the plan is unrelated to the input data types


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] panbingkun commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1234549066


##########
sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala:
##########
@@ -1110,16 +1110,23 @@ class StringFunctionsSuite extends QueryTest with SharedSparkSession {
   }
 
   test("endswith") {
-    val df = Seq(("Spark SQL", "Spark")).toDF("a", "b")
+    val df = Seq(("Spark SQL", "Spark", Array[Byte](1, 2, 3, 4), Array[Byte](3, 4)))
+      .toDF("a", "b", "c", "d")
+
+    checkAnswer(df.selectExpr("endswith(c, d)"), Row(true))
+    checkAnswer(df.select(endswith(col("c"), col("d"))), Row(true))
 
-    checkAnswer(df.selectExpr("endswith(a, b)"), Row(false))
+    // test binary
+    checkAnswer(df.selectExpr("endswith(c, d)"), Row(true))
     checkAnswer(df.select(endswith(col("a"), col("b"))), Row(false))

Review Comment:
   ```suggestion
       checkAnswer(df.select(endswith(col("c"), col("d"))), Row(false))
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] panbingkun commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1234549429


##########
sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala:
##########
@@ -1110,16 +1110,23 @@ class StringFunctionsSuite extends QueryTest with SharedSparkSession {
   }
 
   test("endswith") {
-    val df = Seq(("Spark SQL", "Spark")).toDF("a", "b")
+    val df = Seq(("Spark SQL", "Spark", Array[Byte](1, 2, 3, 4), Array[Byte](3, 4)))
+      .toDF("a", "b", "c", "d")
+
+    checkAnswer(df.selectExpr("endswith(c, d)"), Row(true))
+    checkAnswer(df.select(endswith(col("c"), col("d"))), Row(true))
 
-    checkAnswer(df.selectExpr("endswith(a, b)"), Row(false))
+    // test binary
+    checkAnswer(df.selectExpr("endswith(c, d)"), Row(true))
     checkAnswer(df.select(endswith(col("a"), col("b"))), Row(false))

Review Comment:
   ```suggestion
       checkAnswer(df.select(endswith(col("c"), col("d"))), Row(true))
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] panbingkun commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1234547859


##########
sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala:
##########
@@ -1110,16 +1110,23 @@ class StringFunctionsSuite extends QueryTest with SharedSparkSession {
   }
 
   test("endswith") {
-    val df = Seq(("Spark SQL", "Spark")).toDF("a", "b")
+    val df = Seq(("Spark SQL", "Spark", Array[Byte](1, 2, 3, 4), Array[Byte](3, 4)))
+      .toDF("a", "b", "c", "d")
+
+    checkAnswer(df.selectExpr("endswith(c, d)"), Row(true))
+    checkAnswer(df.select(endswith(col("c"), col("d"))), Row(true))

Review Comment:
   ```suggestion
       checkAnswer(df.select(endswith(col("a"), col("b"))), Row(true))
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zhengruifeng commented on pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #41659:
URL: https://github.com/apache/spark/pull/41659#issuecomment-1598171027

   thank you all, merged to master


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zhengruifeng commented on pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #41659:
URL: https://github.com/apache/spark/pull/41659#issuecomment-1597907324

   > Do we need to update `PlanGenerationTestSuite`?
   > 
   > https://github.com/apache/spark/blob/f3db20c17dfdc1cb5daa42c154afa732e5e3800b/connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/PlanGenerationTestSuite.scala#L2461-L2467
   
   https://github.com/apache/spark/pull/41659#issuecomment-1597862195
   
   I don't think so, since the plan is unrelated to the input data types


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] panbingkun commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1234554772


##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -3947,10 +3947,6 @@ object functions {
    * Returns a boolean. The value is True if str ends with suffix. Returns NULL if either input
    * expression is NULL. Otherwise, returns False. Both str or suffix must be of STRING type.

Review Comment:
   ```suggestion
      * expression is NULL. Otherwise, returns False. Both str or suffix must be of STRING or BINARY type.
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zhengruifeng commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1234564568


##########
sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala:
##########
@@ -1110,16 +1110,23 @@ class StringFunctionsSuite extends QueryTest with SharedSparkSession {
   }
 
   test("endswith") {
-    val df = Seq(("Spark SQL", "Spark")).toDF("a", "b")
+    val df = Seq(("Spark SQL", "Spark", Array[Byte](1, 2, 3, 4), Array[Byte](3, 4)))
+      .toDF("a", "b", "c", "d")
+
+    checkAnswer(df.selectExpr("endswith(c, d)"), Row(true))

Review Comment:
   yeah, I shouldn't change existing tests.
   good catch



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] panbingkun commented on pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #41659:
URL: https://github.com/apache/spark/pull/41659#issuecomment-1597862195

   Do we need to update `PlanGenerationTestSuite`?
   https://github.com/apache/spark/blob/f3db20c17dfdc1cb5daa42c154afa732e5e3800b/connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/PlanGenerationTestSuite.scala#L2461-L2467


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] panbingkun commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1234547859


##########
sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala:
##########
@@ -1110,16 +1110,23 @@ class StringFunctionsSuite extends QueryTest with SharedSparkSession {
   }
 
   test("endswith") {
-    val df = Seq(("Spark SQL", "Spark")).toDF("a", "b")
+    val df = Seq(("Spark SQL", "Spark", Array[Byte](1, 2, 3, 4), Array[Byte](3, 4)))
+      .toDF("a", "b", "c", "d")
+
+    checkAnswer(df.selectExpr("endswith(c, d)"), Row(true))
+    checkAnswer(df.select(endswith(col("c"), col("d"))), Row(true))

Review Comment:
   ```suggestion
       checkAnswer(df.select(endswith(col("a"), col("b"))), Row(false))
   ```



##########
sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala:
##########
@@ -1110,16 +1110,23 @@ class StringFunctionsSuite extends QueryTest with SharedSparkSession {
   }
 
   test("endswith") {
-    val df = Seq(("Spark SQL", "Spark")).toDF("a", "b")
+    val df = Seq(("Spark SQL", "Spark", Array[Byte](1, 2, 3, 4), Array[Byte](3, 4)))
+      .toDF("a", "b", "c", "d")
+
+    checkAnswer(df.selectExpr("endswith(c, d)"), Row(true))

Review Comment:
   ```suggestion
       checkAnswer(df.selectExpr("endswith(a, b)"), Row(false))
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] panbingkun commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1234547795


##########
sql/core/src/test/scala/org/apache/spark/sql/StringFunctionsSuite.scala:
##########
@@ -1110,16 +1110,23 @@ class StringFunctionsSuite extends QueryTest with SharedSparkSession {
   }
 
   test("endswith") {
-    val df = Seq(("Spark SQL", "Spark")).toDF("a", "b")
+    val df = Seq(("Spark SQL", "Spark", Array[Byte](1, 2, 3, 4), Array[Byte](3, 4)))
+      .toDF("a", "b", "c", "d")
+
+    checkAnswer(df.selectExpr("endswith(c, d)"), Row(true))

Review Comment:
   ```suggestion
       checkAnswer(df.selectExpr("endswith(a, b)"), Row(true))
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zhengruifeng commented on pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #41659:
URL: https://github.com/apache/spark/pull/41659#issuecomment-1598061787

   > is there a need to also update the example at https://github.com/apache/spark/blob/master/sql/core/src/test/resources/sql-functions/sql-expression-schema.md to indicate binary type is supported?
   
   I think it's not needed, since these PRs don't touch SQL syntax at all.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zhengruifeng commented on pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #41659:
URL: https://github.com/apache/spark/pull/41659#issuecomment-1598062051

   > LGTM except one comment @panbingkun left.
   
   I think @panbingkun's comments have already been addressed.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] beliefer commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1233981865


##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -3947,10 +3947,6 @@ object functions {
    * Returns a boolean. The value is True if str ends with suffix. Returns NULL if either input
    * expression is NULL. Otherwise, returns False. Both str or suffix must be of STRING type.

Review Comment:
   The comment need update now.



##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -4048,31 +4048,27 @@ object functions {
    * Returns NULL if either input expression is NULL. Otherwise, returns False.
    * Both str or suffix must be of STRING type.
    *
-   * @note
-   *   Only STRING type is supported in this function, while `endswith` in SQL supports both
-   *   STRING and BINARY.
-   *
    * @group string_funcs
    * @since 3.5.0
    */
-  def endswith(str: Column, suffix: Column): Column = withExpr {
-    EndsWith(str.expr, suffix.expr)
+  def endswith(str: Column, suffix: Column): Column = {
+    // 'EndsWith' expression only supports StringType,
+    // use 'call_udf' to support both StringType and BinaryType.
+    call_udf("endswith", str, suffix)

Review Comment:
   I see the usage for call_udf.



##########
sql/core/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -4048,31 +4048,27 @@ object functions {
    * Returns NULL if either input expression is NULL. Otherwise, returns False.
    * Both str or suffix must be of STRING type.

Review Comment:
   ditto.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zhengruifeng closed pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type
URL: https://github.com/apache/spark/pull/41659


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zhengruifeng commented on pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #41659:
URL: https://github.com/apache/spark/pull/41659#issuecomment-1596837656

   cc @beliefer @panbingkun @HyukjinKwon 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zhengruifeng commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1234001719


##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -3947,10 +3947,6 @@ object functions {
    * Returns a boolean. The value is True if str ends with suffix. Returns NULL if either input
    * expression is NULL. Otherwise, returns False. Both str or suffix must be of STRING type.

Review Comment:
   good catch



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] panbingkun commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1234554772


##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -3947,10 +3947,6 @@ object functions {
    * Returns a boolean. The value is True if str ends with suffix. Returns NULL if either input
    * expression is NULL. Otherwise, returns False. Both str or suffix must be of STRING type.

Review Comment:
   ```suggestion
      * expression is NULL. Otherwise, returns False. Both left or right must be of STRING or BINARY type.
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] panbingkun commented on a diff in pull request #41659: [SPARK-43944][SQL][CONNECT][PYTHON][FOLLOW-UP] Make `startswith` & `endswith` support binary type

Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41659:
URL: https://github.com/apache/spark/pull/41659#discussion_r1234555175


##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -3961,10 +3957,6 @@ object functions {
    * Returns a boolean. The value is True if str starts with prefix. Returns NULL if either input
    * expression is NULL. Otherwise, returns False. Both str or prefix must be of STRING type.

Review Comment:
   ```suggestion
      * expression is NULL. Otherwise, returns False. Both str or prefix must be of STRING or BINARY type..
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org