You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "beliefer (via GitHub)" <gi...@apache.org> on 2024/06/12 12:56:19 UTC
[PR] [SPARK-48584][SQL][FOLLOWUP] Improve the unescapePathName. [spark]
beliefer opened a new pull request, #46957:
URL: https://github.com/apache/spark/pull/46957
### What changes were proposed in this pull request?
This PR follows up https://github.com/apache/spark/pull/46938 and improve the `unescapePathName`.
### Why are the changes needed?
Improve the `unescapePathName` by cut off slow path.
### Does this PR introduce _any_ user-facing change?
'No'.
### How was this patch tested?
GA.
### Was this patch authored or co-authored using generative AI tooling?
'No'.
--
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
Re: [PR] [SPARK-48584][SQL][FOLLOWUP] Improve the unescapePathName. [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #46957:
URL: https://github.com/apache/spark/pull/46957#discussion_r1637464645
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogUtils.scala:
##########
@@ -103,13 +103,13 @@ object ExternalCatalogUtils {
}
var plaintextEndIdx = path.indexOf('%')
val length = path.length
- if (plaintextEndIdx == -1 || plaintextEndIdx + 2 > length) {
+ if (plaintextEndIdx == -1 || plaintextEndIdx + 2 >= length) {
// fast path, no %xx encoding found then return the string identity
path
} else {
val sb = new java.lang.StringBuilder(length)
var plaintextStartIdx = 0
- while(plaintextEndIdx != -1 && plaintextEndIdx + 2 < length) {
+ do {
Review Comment:
Thank you for the remind. I will revert 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
Re: [PR] [SPARK-48584][SQL][FOLLOWUP] Improve the unescapePathName. [spark]
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #46957:
URL: https://github.com/apache/spark/pull/46957#discussion_r1637458005
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogUtils.scala:
##########
@@ -103,13 +103,13 @@ object ExternalCatalogUtils {
}
var plaintextEndIdx = path.indexOf('%')
val length = path.length
- if (plaintextEndIdx == -1 || plaintextEndIdx + 2 > length) {
+ if (plaintextEndIdx == -1 || plaintextEndIdx + 2 >= length) {
// fast path, no %xx encoding found then return the string identity
path
} else {
val sb = new java.lang.StringBuilder(length)
var plaintextStartIdx = 0
- while(plaintextEndIdx != -1 && plaintextEndIdx + 2 < length) {
+ do {
Review Comment:
Yes, what @yaooqinn said is correct:
```scala
Welcome to Scala 3.3.1 (17.0.11, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
scala> var x = 10
|
| do {
| println("Value of x: " + x)
| x = x - 1
| } while (x > 0)
-- Error: ----------------------------------------------------------------------
3 |do {
|^^
|end of statement expected but 'do' found
```
--
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
Re: [PR] [SPARK-48584][SQL][FOLLOWUP] Improve the unescapePathName. [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #46957:
URL: https://github.com/apache/spark/pull/46957#discussion_r1637432947
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/ExternalCatalogUtils.scala:
##########
@@ -103,13 +103,13 @@ object ExternalCatalogUtils {
}
var plaintextEndIdx = path.indexOf('%')
val length = path.length
- if (plaintextEndIdx == -1 || plaintextEndIdx + 2 > length) {
+ if (plaintextEndIdx == -1 || plaintextEndIdx + 2 >= length) {
// fast path, no %xx encoding found then return the string identity
path
} else {
val sb = new java.lang.StringBuilder(length)
var plaintextStartIdx = 0
- while(plaintextEndIdx != -1 && plaintextEndIdx + 2 < length) {
+ do {
Review Comment:
FYI, for scala 3, do-while is removed
--
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
Re: [PR] [SPARK-48584][SQL][FOLLOWUP] Improve the unescapePathName. [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #46957:
URL: https://github.com/apache/spark/pull/46957#issuecomment-2164256089
ping @yaooqinn @LuciferYang
--
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
Re: [PR] [SPARK-48584][SQL][FOLLOWUP] Improve the unescapePathName. [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on PR #46957:
URL: https://github.com/apache/spark/pull/46957#issuecomment-2164647642
Merged to master. Thank you @yaooqinn @LuciferYang
--
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
Re: [PR] [SPARK-48584][SQL][FOLLOWUP] Improve the unescapePathName. [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer closed pull request #46957: [SPARK-48584][SQL][FOLLOWUP] Improve the unescapePathName.
URL: https://github.com/apache/spark/pull/46957
--
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