You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "panbingkun (via GitHub)" <gi...@apache.org> on 2023/05/24 07:02:27 UTC
[GitHub] [spark] panbingkun opened a new pull request, #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, max_distance])' functions
panbingkun opened a new pull request, #41293:
URL: https://github.com/apache/spark/pull/41293
### What changes were proposed in this pull request?
The pr aims to implement 'levenshtein(str1, str2[, max_distance])' functions for `connect` module.
### Why are the changes needed?
After [Add a max distance argument to the levenshtein() function](https://issues.apache.org/jira/browse/SPARK-43493) We have already implemented it on the scala side, so we need to align it.
### Does this PR introduce _any_ user-facing change?
Yes, new API for Connect.
### How was this patch tested?
- Pass GA.
- Manual testing
1../build/sbt "connect-client-jvm/testOnly *ClientE2ETestSuite*"
2.sh dev/connect-jvm-client-mima-check
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204899744
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -2896,6 +2896,16 @@ object functions {
*/
def levenshtein(l: Column, r: Column): Column = Column.fn("levenshtein", l, r)
+ /**
+ * Computes the Levenshtein distance of the two given string columns if it's less than or
+ * equal to a given threshold.
+ * @return result distance, or -1
Review Comment:
When the `actual` Levenshtein distance between the two given strings > `threshold`, it return -1.
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #41293:
URL: https://github.com/apache/spark/pull/41293#issuecomment-1562131002
The implementation of `Python` & `connect Python` is: https://github.com/apache/spark/pull/41296,
After it, I will continue to work on above.
@hvanhovell
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204879109
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -801,6 +801,19 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
assert(plan.sameSemantics(otherPlan))
}
+ test("levenshtein") {
Review Comment:
Ok, let me remove it
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #41293:
URL: https://github.com/apache/spark/pull/41293#issuecomment-1562536921
> is this one ready to go
Yes
--
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] LuciferYang commented on pull request #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #41293:
URL: https://github.com/apache/spark/pull/41293#issuecomment-1562520371
is this one ready to go? @panbingkun
--
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] LuciferYang commented on pull request #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #41293:
URL: https://github.com/apache/spark/pull/41293#issuecomment-1562520041
@panbingkun
Before submitting your change, please make sure to format your code using the following command:
./build/mvn -Pscala-2.12 scalafmt:format -Dscalafmt.skip=false -Dscalafmt.validateOnly=false -Dscalafmt.changedOnly=false -pl connector/connect/common -pl connector/connect/server -pl connector/connect/client/jvm
Error: Process completed with exit code 1.
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #41293:
URL: https://github.com/apache/spark/pull/41293#issuecomment-1563703184
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] panbingkun commented on a diff in pull request #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204927855
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -2896,6 +2896,16 @@ object functions {
*/
def levenshtein(l: Column, r: Column): Column = Column.fn("levenshtein", l, r)
+ /**
+ * Computes the Levenshtein distance of the two given string columns if it's less than or
+ * equal to a given threshold.
+ * @return result distance, or -1
Review Comment:
Let me add UT for it?
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #41293:
URL: https://github.com/apache/spark/pull/41293#issuecomment-1562533084
> ./build/mvn -Pscala-2.12 scalafmt:format -Dscalafmt.skip=false -Dscalafmt.validateOnly=false -Dscalafmt.changedOnly=false -pl connector/connect/common -pl connector/connect/server -pl connector/connect/client/jvm
OK
--
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] hvanhovell commented on a diff in pull request #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204385839
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -801,6 +801,19 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
assert(plan.sameSemantics(otherPlan))
}
+ test("levenshtein") {
Review Comment:
+1
--
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] LuciferYang commented on a diff in pull request #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204321957
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -801,6 +801,19 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
assert(plan.sameSemantics(otherPlan))
}
+ test("levenshtein") {
Review Comment:
The test is ok, but it may not be necessary to add this e2e test, as other functions don't have e2e tests now. Add new test to PlanGenerationTestSuite is enough
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204927855
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -2896,6 +2896,16 @@ object functions {
*/
def levenshtein(l: Column, r: Column): Column = Column.fn("levenshtein", l, r)
+ /**
+ * Computes the Levenshtein distance of the two given string columns if it's less than or
+ * equal to a given threshold.
+ * @return result distance, or -1
Review Comment:
Let me add UT for it.
PR: https://github.com/apache/spark/pull/41303
@zhengruifeng
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204882175
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -2896,6 +2896,16 @@ object functions {
*/
def levenshtein(l: Column, r: Column): Column = Column.fn("levenshtein", l, r)
+ /**
+ * Computes the Levenshtein distance of the two given string columns if it's less than or
+ * equal to a given threshold.
+ * @return result distance, or -1
Review Comment:
When any argument of the xxx function is null, it will return null.
BTW, function levenshtein inherits NullIntolerant.
https://github.com/apache/spark/blob/5ec138546205ba4248cc9ec72c3b7baf60f2fede/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala#L2150-L2156
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
URL: https://github.com/apache/spark/pull/41293
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204882175
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -2896,6 +2896,16 @@ object functions {
*/
def levenshtein(l: Column, r: Column): Column = Column.fn("levenshtein", l, r)
+ /**
+ * Computes the Levenshtein distance of the two given string columns if it's less than or
+ * equal to a given threshold.
+ * @return result distance, or -1
Review Comment:
When any argument of the levenshtein function is null, it will return null.
BTW, function levenshtein inherits NullIntolerant.
https://github.com/apache/spark/blob/5ec138546205ba4248cc9ec72c3b7baf60f2fede/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala#L2150-L2156
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204907749
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -2896,6 +2896,16 @@ object functions {
*/
def levenshtein(l: Column, r: Column): Column = Column.fn("levenshtein", l, r)
+ /**
+ * Computes the Levenshtein distance of the two given string columns if it's less than or
+ * equal to a given threshold.
+ * @return result distance, or -1
Review Comment:
```
scala> df.select($"l", $"r", levenshtein($"l", $"r"), levenshtein($"l", $"r", 2), levenshtein($"l", $"r", 3)).show()
+------+-------+-----------------+--------------------+--------------------+
| l| r|levenshtein(l, r)|levenshtein(l, r, 2)|levenshtein(l, r, 3)|
+------+-------+-----------------+--------------------+--------------------+
|kitten|sitting| 3| -1| 3|
| frog| NULL| NULL| NULL| NULL|
+------+-------+-----------------+--------------------+--------------------+
```
it seems it returns -1 if inputs are not-null and the disttance > threshold.
but we don't have a test for nullability in `StringFunctionsSuite`
--
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] hvanhovell commented on a diff in pull request #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204892930
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -2896,6 +2896,16 @@ object functions {
*/
def levenshtein(l: Column, r: Column): Column = Column.fn("levenshtein", l, r)
+ /**
+ * Computes the Levenshtein distance of the two given string columns if it's less than or
+ * equal to a given threshold.
+ * @return result distance, or -1
Review Comment:
So when does it return -1?
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #41293:
URL: https://github.com/apache/spark/pull/41293#issuecomment-1562099061
> Thanks for doing this! Are you also taking care of python?
I want to try it. 😄
--
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] hvanhovell commented on a diff in pull request #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #41293:
URL: https://github.com/apache/spark/pull/41293#discussion_r1204387620
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/functions.scala:
##########
@@ -2896,6 +2896,16 @@ object functions {
*/
def levenshtein(l: Column, r: Column): Column = Column.fn("levenshtein", l, r)
+ /**
+ * Computes the Levenshtein distance of the two given string columns if it's less than or
+ * equal to a given threshold.
+ * @return result distance, or -1
Review Comment:
Do we return -1 on null? If so, then we should consider returning null (if we can).
--
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 #41293: [SPARK-43769][CONNECT] Implement 'levenshtein(str1, str2[, threshold])' functions
Posted by "panbingkun (via GitHub)" <gi...@apache.org>.
panbingkun commented on PR #41293:
URL: https://github.com/apache/spark/pull/41293#issuecomment-1560575635
cc @hvanhovell @zhengruifeng
--
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