You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/10/18 08:00:41 UTC
[GitHub] [spark] LuciferYang opened a new pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
LuciferYang opened a new pull request #34313:
URL: https://github.com/apache/spark/pull/34313
### What changes were proposed in this pull request?
The following sql has different behavior when using Java 8 and Java 17
```
--PostgreSQL throw ERROR: format specifies argument 0, but arguments are numbered from 1
select format_string('%0$s', 'Hello');
```
**Use Java 8**
```
-- !query
select format_string('%0$s', 'Hello')
-- !query schema
struct<format_string(%0$s, Hello):string>
-- !query output
Hello
```
**Use Java 17**
```
-- !query
select format_string('%0$s', 'Hello')
-- !query schema
struct<>
-- !query output
java.util.IllegalFormatArgumentIndexException
Illegal format argument index = 0
```
The difference in this behavior comes from the change of `j.u.Formatter.FormatSpecifier.index` method:
Java 8
```
private int index(String s) {
if (s != null) {
try {
index = Integer.parseInt(s.substring(0, s.length() - 1));
} catch (NumberFormatException x) {
assert(false);
}
} else {
index = 0;
}
return index;
}
```
Java 17
```java
private void index(String s, int start, int end) {
if (start >= 0) {
try {
// skip the trailing '$'
index = Integer.parseInt(s, start, end - 1, 10);
if (index <= 0) {
throw new IllegalFormatArgumentIndexException(index);
}
} catch (NumberFormatException x) {
throw new IllegalFormatArgumentIndexException(Integer.MIN_VALUE);
}
}
}
```
A `index <= 0` condition is added here to ensure `%0$` as a `IllegalFormatArgumentIndexException` expression.
So the main change of this pr is add the manually replacing `%0$` with `%1$` to ensure that Java 17 and Java 8 have the same behavior.
### Why are the changes needed?
Pass UT with JDK 17
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
- Pass the Jenkins or GitHub Action
--
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] AmplabJenkins commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948344638
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48963/
--
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 #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-949302214
thanks 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] LuciferYang commented on a change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r737154105
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1617,6 +1617,8 @@ case class ParseUrl(children: Seq[Expression], failOnError: Boolean = SQLConf.ge
case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes {
require(children.nonEmpty, s"$prettyName() should take at least 1 argument")
+ require(checkArgumentIndexNotZero(children(0)), "Illegal format argument index = 0")
Review comment:
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] AmplabJenkins commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947625315
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144449/
--
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] AmplabJenkins removed a comment on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947678898
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144453/
--
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] AmplabJenkins removed a comment on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-945630844
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48835/
--
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] AmplabJenkins removed a comment on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947625315
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144449/
--
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] SparkQA removed a comment on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-945532953
**[Test build #144360 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144360/testReport)** for PR 34313 at commit [`52aa467`](https://github.com/apache/spark/commit/52aa46781048e6284924e0f3f46733e3652720de).
--
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 #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947289266
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] wangyum commented on a change in pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r732523697
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1701,13 +1701,9 @@ case class FormatString(children: Expression*) extends Expression with ImplicitC
* Therefore, manually check that the pattern string not contains "%0$" to ensure consistent
* behavior of Java 8, Java 11 and Java 17.
*/
- private def checkArgumentIndexNotZero(expression: Expression): Boolean = {
- val pattern = expression.eval(null)
- if (pattern == null) {
- true
- } else {
- !pattern.asInstanceOf[UTF8String].toString.contains("%0$")
- }
+ private def checkArgumentIndexNotZero(expression: Expression): Boolean = expression match {
+ case pattern: Literal if pattern.dataType == StringType => !pattern.toString.contains("%0$")
Review comment:
`case StringLiteral(pattern) => !pattern.contains("%0$")`?
--
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 change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r737118939
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1617,6 +1617,8 @@ case class ParseUrl(children: Seq[Expression], failOnError: Boolean = SQLConf.ge
case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes {
require(children.nonEmpty, s"$prettyName() should take at least 1 argument")
+ require(checkArgumentIndexNotZero(children(0)), "Illegal format argument index = 0")
Review comment:
@cloud-fan Sorry, I didn't pay attention to this framework, is there any sample? I'll fix it later
--
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] AmplabJenkins commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-945630844
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48835/
--
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] SparkQA removed a comment on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947411360
**[Test build #144449 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144449/testReport)** for PR 34313 at commit [`c57b424`](https://github.com/apache/spark/commit/c57b424310ffba24dabc3b77afcbe45324a84a72).
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947601398
**[Test build #144449 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144449/testReport)** for PR 34313 at commit [`c57b424`](https://github.com/apache/spark/commit/c57b424310ffba24dabc3b77afcbe45324a84a72).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948306729
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48963/
--
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] AmplabJenkins removed a comment on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948529538
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144491/
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948500775
**[Test build #144491 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144491/testReport)** for PR 34313 at commit [`ba7c160`](https://github.com/apache/spark/commit/ba7c1602918a2ab28de4ff63f789d06081cbd807).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947496160
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48926/
--
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] wangyum commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948174827
Do we need to add an item in `docs/sql-migration-guide.md`? since it's a breaking change for Java 8/11 user.
--
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] AmplabJenkins commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948529538
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144491/
--
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] srowen closed pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
srowen closed pull request #34313:
URL: https://github.com/apache/spark/pull/34313
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-945630807
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48835/
--
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 change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r737128931
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1617,6 +1617,8 @@ case class ParseUrl(children: Seq[Expression], failOnError: Boolean = SQLConf.ge
case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes {
require(children.nonEmpty, s"$prettyName() should take at least 1 argument")
+ require(checkArgumentIndexNotZero(children(0)), "Illegal format argument index = 0")
Review comment:
@cloud-fan Like this PR? https://github.com/apache/spark/pull/34208/files ?
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-945773201
**[Test build #144360 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144360/testReport)** for PR 34313 at commit [`52aa467`](https://github.com/apache/spark/commit/52aa46781048e6284924e0f3f46733e3652720de).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Forbid '%0$' explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947453558
**[Test build #144453 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144453/testReport)** for PR 34313 at commit [`7e8c3d6`](https://github.com/apache/spark/commit/7e8c3d694222d3cb46c1a56d402256d7deb234dd).
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947411360
**[Test build #144449 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144449/testReport)** for PR 34313 at commit [`c57b424`](https://github.com/apache/spark/commit/c57b424310ffba24dabc3b77afcbe45324a84a72).
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947530757
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48926/
--
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] srowen commented on a change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r732800414
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1688,6 +1690,21 @@ case class FormatString(children: Expression*) extends Expression with ImplicitC
override protected def withNewChildrenInternal(
newChildren: IndexedSeq[Expression]): FormatString = FormatString(newChildren: _*)
+
+ /**
+ * SPARK-37013: The `formatSpecifier` defined in `j.u.Formatter` as follows:
+ * "%[argument_index$][flags][width][.precision][t]conversion"
+ * The optional `argument_index` is a decimal integer indicating the position of the argument
+ * in the argument list. The first argument is referenced by "1$", the second by "2$", etc.
+ * However, for the illegal definition of "%0$", Java 8 and Java 11 uses it as "%1$",
+ * and Java 17 throws IllegalFormatArgumentIndexException(Illegal format argument index = 0).
+ * Therefore, manually check that the pattern string not contains "%0$" to ensure consistent
+ * behavior of Java 8, Java 11 and Java 17.
+ */
+ private def checkArgumentIndexNotZero(expression: Expression): Boolean = expression match {
+ case StringLiteral(pattern) => !pattern.contains("%0$")
Review comment:
Ah OK it goes after $ - seems OK then
--
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 change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r732797793
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1688,6 +1690,21 @@ case class FormatString(children: Expression*) extends Expression with ImplicitC
override protected def withNewChildrenInternal(
newChildren: IndexedSeq[Expression]): FormatString = FormatString(newChildren: _*)
+
+ /**
+ * SPARK-37013: The `formatSpecifier` defined in `j.u.Formatter` as follows:
+ * "%[argument_index$][flags][width][.precision][t]conversion"
+ * The optional `argument_index` is a decimal integer indicating the position of the argument
+ * in the argument list. The first argument is referenced by "1$", the second by "2$", etc.
+ * However, for the illegal definition of "%0$", Java 8 and Java 11 uses it as "%1$",
+ * and Java 17 throws IllegalFormatArgumentIndexException(Illegal format argument index = 0).
+ * Therefore, manually check that the pattern string not contains "%0$" to ensure consistent
+ * behavior of Java 8, Java 11 and Java 17.
+ */
+ private def checkArgumentIndexNotZero(expression: Expression): Boolean = expression match {
+ case StringLiteral(pattern) => !pattern.contains("%0$")
Review comment:
The `formatSpecifier` defined in `j.u.Formatter` is `%[argument_index$][flags][width][.precision][t]conversion`, from the definition of this format, can we think that there will be no other content between `%` and `0$`?
--
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] SparkQA removed a comment on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948280989
**[Test build #144491 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144491/testReport)** for PR 34313 at commit [`ba7c160`](https://github.com/apache/spark/commit/ba7c1602918a2ab28de4ff63f789d06081cbd807).
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948343025
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48963/
--
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 change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r737118939
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1617,6 +1617,8 @@ case class ParseUrl(children: Seq[Expression], failOnError: Boolean = SQLConf.ge
case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes {
require(children.nonEmpty, s"$prettyName() should take at least 1 argument")
+ require(checkArgumentIndexNotZero(children(0)), "Illegal format argument index = 0")
Review comment:
@cloud-fan Is there any sample? I'll fix it later
--
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 change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r738915950
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1617,6 +1617,8 @@ case class ParseUrl(children: Seq[Expression], failOnError: Boolean = SQLConf.ge
case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes {
require(children.nonEmpty, s"$prettyName() should take at least 1 argument")
+ require(checkArgumentIndexNotZero(children(0)), "Illegal format argument index = 0")
Review comment:
I'm very sorry I haven't started this week
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-945582376
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48835/
--
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] srowen commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-946876750
I'm kinda confused - seems like using index 0 was always disallowed: https://docs.oracle.com/javase/8/docs/api/java/util/Formatter.html
"The first argument is referenced by "1$", the second by "2$", etc."
I wonder how it ever worked or what it did? If it was illegal to begin with, we could just let Java 17 enforce it. I'm OK with explicitly enforcing it in order to avoid behavior difference across JVMs for the same Spark version too. I would not edit the format string, I think.
Wouldn't all the numbered indices have to change anyway? otherwise editing the string might result in two arguments at position 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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947669995
**[Test build #144453 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144453/testReport)** for PR 34313 at commit [`7e8c3d6`](https://github.com/apache/spark/commit/7e8c3d694222d3cb46c1a56d402256d7deb234dd).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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] SparkQA removed a comment on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947453558
**[Test build #144453 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144453/testReport)** for PR 34313 at commit [`7e8c3d6`](https://github.com/apache/spark/commit/7e8c3d694222d3cb46c1a56d402256d7deb234dd).
--
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] AmplabJenkins commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947678898
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144453/
--
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] AmplabJenkins removed a comment on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947498176
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48922/
--
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] wangyum edited a comment on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
wangyum edited a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948174827
Do we need to add an item in `docs/sql-migration-guide.md`? since it's a breaking change for Java 8/11 users.
--
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] dongjoon-hyun commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947343562
+1 for @srowen 's comment and I prefer to forbid explicitly because it's consistent with PostgreSQL too.
--
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 #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947283510
We can use the following case to test it
```
import java.util.Locale
import java.util.Formatter
val sb = new StringBuffer()
// Send all output to the Appendable object sb
val formatter = new Formatter(sb, Locale.US)
// Explicit argument indices may be used to re-order output.
formatter.format("%4$2s %3$2s %2$2s %1$2s", "a", "b", "c", "d")
assert(sb.toString == " d c b a")
sb.delete(0, sb.length())
assert(sb.toString.isEmpty)
formatter.format("%4$2s %3$2s %2$2s %0$2s", "a", "b", "c", "d")
assert(sb.toString == " d c b a")
```
This case is similar to the example written in the https://docs.oracle.com/javase/8/docs/api/java/util/Formatter.html
The new case tests both `%4$2s %3$2s %2$2s %1$2s` and `%4$2s %3$2s %2$2s %0$2s` and they passed when use Java 8, although "The first argument is referenced by "1$", the second by "2$", etc." is specified in the document, but `%0$` is not prohibited, and the result of `%0$` is equivalent to `%1$`.
The result of running the above case using Java 11 is the same as that of Java 8.
And Exceptions are thrown only when the above case is run using Java 17
```
Illegal format argument index = 0
java.util.IllegalFormatArgumentIndexException: Illegal format argument index = 0
at java.base/java.util.Formatter$FormatSpecifier.index(Formatter.java:2808)
at java.base/java.util.Formatter$FormatSpecifier.<init>(Formatter.java:2879)
at java.base/java.util.Formatter.parse(Formatter.java:2747)
at java.base/java.util.Formatter.format(Formatter.java:2671)
at java.base/java.util.Formatter.format(Formatter.java:2625)
```
--
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] cloud-fan commented on a change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r737073146
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1617,6 +1617,8 @@ case class ParseUrl(children: Seq[Expression], failOnError: Boolean = SQLConf.ge
case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes {
require(children.nonEmpty, s"$prettyName() should take at least 1 argument")
+ require(checkArgumentIndexNotZero(children(0)), "Illegal format argument index = 0")
Review comment:
let's use the new error framework to throw error in newly added code.
--
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 change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r737128931
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1617,6 +1617,8 @@ case class ParseUrl(children: Seq[Expression], failOnError: Boolean = SQLConf.ge
case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes {
require(children.nonEmpty, s"$prettyName() should take at least 1 argument")
+ require(checkArgumentIndexNotZero(children(0)), "Illegal format argument index = 0")
Review comment:
@cloud-fan like https://github.com/apache/spark/pull/34208/files ?
--
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 change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r737118939
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1617,6 +1617,8 @@ case class ParseUrl(children: Seq[Expression], failOnError: Boolean = SQLConf.ge
case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes {
require(children.nonEmpty, s"$prettyName() should take at least 1 argument")
+ require(checkArgumentIndexNotZero(children(0)), "Illegal format argument index = 0")
Review comment:
@cloud-fan Sorry, is there any sample? I'll fix it later
--
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] AmplabJenkins commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947543091
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48926/
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-945532953
**[Test build #144360 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144360/testReport)** for PR 34313 at commit [`52aa467`](https://github.com/apache/spark/commit/52aa46781048e6284924e0f3f46733e3652720de).
--
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] srowen commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947283900
Right so I support either leaving it to error out on Java 17, or explicitly forbidding it in Spark for consistency
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947448674
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48922/
--
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] cloud-fan commented on a change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r737144189
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1617,6 +1617,8 @@ case class ParseUrl(children: Seq[Expression], failOnError: Boolean = SQLConf.ge
case class FormatString(children: Expression*) extends Expression with ImplicitCastInputTypes {
require(children.nonEmpty, s"$prettyName() should take at least 1 argument")
+ require(checkArgumentIndexNotZero(children(0)), "Illegal format argument index = 0")
Review comment:
yea
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947482417
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/48922/
--
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 change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r732816638
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1688,6 +1690,21 @@ case class FormatString(children: Expression*) extends Expression with ImplicitC
override protected def withNewChildrenInternal(
newChildren: IndexedSeq[Expression]): FormatString = FormatString(newChildren: _*)
+
+ /**
+ * SPARK-37013: The `formatSpecifier` defined in `j.u.Formatter` as follows:
+ * "%[argument_index$][flags][width][.precision][t]conversion"
+ * The optional `argument_index` is a decimal integer indicating the position of the argument
+ * in the argument list. The first argument is referenced by "1$", the second by "2$", etc.
+ * However, for the illegal definition of "%0$", Java 8 and Java 11 uses it as "%1$",
+ * and Java 17 throws IllegalFormatArgumentIndexException(Illegal format argument index = 0).
+ * Therefore, manually check that the pattern string not contains "%0$" to ensure consistent
+ * behavior of Java 8, Java 11 and Java 17.
+ */
+ private def checkArgumentIndexNotZero(expression: Expression): Boolean = expression match {
+ case StringLiteral(pattern) => !pattern.contains("%0$")
Review comment:
> Ah OK it goes after $ - seems OK then
I'm also trying to create cases that may cause misjudgment or missing judgment, but I haven't found them yet.
--
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] AmplabJenkins commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947498176
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48922/
--
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 #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-945484637
I think there are three ways to fix this issue
1. Make Java 17 compatible with Java 8, just as this pr does
2. Using the behavior of Java 17, add judgment `FormatString` function to prohibit the use of `%0$` pattern like
```
if (pattern.contains("%0$")) {
throw new IllegalArgumentException
}
```
and it looks more like what is expected in the test comment: `PostgreSQL throw ERROR: format specifies argument 0, but arguments are numbered from 1`
3. Maintain different behaviors and change the test case, just as https://github.com/apache/spark/pull/34153/files# does
cc @srowen @wangyum @dongjoon-hyun @MaxGekk
--
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] AmplabJenkins removed a comment on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-945775077
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144360/
--
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 #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947267631
> I'm kinda confused - seems like using index 0 was always disallowed: https://docs.oracle.com/javase/8/docs/api/java/util/Formatter.html
"The first argument is referenced by "1$", the second by "2$", etc."
Let me check this
--
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 change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r732808594
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1688,6 +1690,21 @@ case class FormatString(children: Expression*) extends Expression with ImplicitC
override protected def withNewChildrenInternal(
newChildren: IndexedSeq[Expression]): FormatString = FormatString(newChildren: _*)
+
+ /**
+ * SPARK-37013: The `formatSpecifier` defined in `j.u.Formatter` as follows:
+ * "%[argument_index$][flags][width][.precision][t]conversion"
+ * The optional `argument_index` is a decimal integer indicating the position of the argument
+ * in the argument list. The first argument is referenced by "1$", the second by "2$", etc.
+ * However, for the illegal definition of "%0$", Java 8 and Java 11 uses it as "%1$",
+ * and Java 17 throws IllegalFormatArgumentIndexException(Illegal format argument index = 0).
+ * Therefore, manually check that the pattern string not contains "%0$" to ensure consistent
+ * behavior of Java 8, Java 11 and Java 17.
+ */
+ private def checkArgumentIndexNotZero(expression: Expression): Boolean = expression match {
+ case StringLiteral(pattern) => !pattern.contains("%0$")
Review comment:
Or we can move part of the `j.u.Formatter.parse()` method code from Java 17 to make more strict 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] LuciferYang commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948280713
@wangyum done
--
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] SparkQA commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948280989
**[Test build #144491 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144491/testReport)** for PR 34313 at commit [`ba7c160`](https://github.com/apache/spark/commit/ba7c1602918a2ab28de4ff63f789d06081cbd807).
--
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] srowen commented on a change in pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #34313:
URL: https://github.com/apache/spark/pull/34313#discussion_r732791348
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
##########
@@ -1688,6 +1690,21 @@ case class FormatString(children: Expression*) extends Expression with ImplicitC
override protected def withNewChildrenInternal(
newChildren: IndexedSeq[Expression]): FormatString = FormatString(newChildren: _*)
+
+ /**
+ * SPARK-37013: The `formatSpecifier` defined in `j.u.Formatter` as follows:
+ * "%[argument_index$][flags][width][.precision][t]conversion"
+ * The optional `argument_index` is a decimal integer indicating the position of the argument
+ * in the argument list. The first argument is referenced by "1$", the second by "2$", etc.
+ * However, for the illegal definition of "%0$", Java 8 and Java 11 uses it as "%1$",
+ * and Java 17 throws IllegalFormatArgumentIndexException(Illegal format argument index = 0).
+ * Therefore, manually check that the pattern string not contains "%0$" to ensure consistent
+ * behavior of Java 8, Java 11 and Java 17.
+ */
+ private def checkArgumentIndexNotZero(expression: Expression): Boolean = expression match {
+ case StringLiteral(pattern) => !pattern.contains("%0$")
Review comment:
I think there may be more ways to use index 0 - what if you have something between the % and 0$?
This might in practice be close enough, just wondering if there is a simple way to make this more comprehensive without false positives
--
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] AmplabJenkins removed a comment on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-948344638
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48963/
--
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 edited a comment on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
LuciferYang edited a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947267631
> I'm kinda confused - seems like using index 0 was always disallowed: https://docs.oracle.com/javase/8/docs/api/java/util/Formatter.html
"The first argument is referenced by "1$", the second by "2$", etc."
Let me double check this
--
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] AmplabJenkins removed a comment on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-947543091
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/48926/
--
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] AmplabJenkins commented on pull request #34313: [SPARK-37013][SQL] Ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-945775077
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144360/
--
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] srowen commented on pull request #34313: [SPARK-37013][SQL] Forbid `%0$` usage explicitly to ensure `format_string` has same behavior when using Java 8 and Java 17
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #34313:
URL: https://github.com/apache/spark/pull/34313#issuecomment-949284674
I copied the release notes change to the JIRA too. I'll merge 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