You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by wangyum <gi...@git.apache.org> on 2018/09/23 15:55:20 UTC
[GitHub] spark pull request #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT whe...
GitHub user wangyum opened a pull request:
https://github.com/apache/spark/pull/22531
[SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpperCase
## What changes were proposed in this pull request?
Add `Locale.ROOT` when `toUpperCase`.
## How was this patch tested?
manual tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/wangyum/spark SPARK-25415
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22531.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #22531
----
commit d138427f35d4980b263dfef21b8810fe455443ca
Author: Yuming Wang <yu...@...>
Date: 2018-09-23T15:51:53Z
Add Locale.ROOT when toUpperCase
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on the issue:
https://github.com/apache/spark/pull/22531
Thanks @HyukjinKwon I'd love to do it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22531
**[Test build #96493 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96493/testReport)** for PR 22531 at commit [`d138427`](https://github.com/apache/spark/commit/d138427f35d4980b263dfef21b8810fe455443ca).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22531
Thanks, will open a PR shortly.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22531
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3390/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22531
**[Test build #96490 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96490/testReport)** for PR 22531 at commit [`d138427`](https://github.com/apache/spark/commit/d138427f35d4980b263dfef21b8810fe455443ca).
* This patch **fails SparkR unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22531
Oh, you mean @seancxmao is going to take over? Let me open a PR directly since I have some changes in local if you don't mind.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22531
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96490/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22531
Thank you @seancxmao.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22531
@HyukjinKwon I think it's OK to fix these instances, yes. We have to be careful to restrict it to cases where the locale definitely should not matter, like when comparing enum values and other program identifiers.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on the issue:
https://github.com/apache/spark/pull/22531
@HyukjinKwon @seancxmao will take it over: [SPARK-25565](https://issues.apache.org/jira/browse/SPARK-25565)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22531
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22531
@wangyum, are you interested in submitting a PR to check if we can add a rule for `.toLowerCase(Locale.ROOT)` and `.toUpperCase(Locale.ROOT)` and add it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22531
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT whe...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22531
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22531
**[Test build #96493 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96493/testReport)** for PR 22531 at commit [`d138427`](https://github.com/apache/spark/commit/d138427f35d4980b263dfef21b8810fe455443ca).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22531
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96493/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by seancxmao <gi...@git.apache.org>.
Github user seancxmao commented on the issue:
https://github.com/apache/spark/pull/22531
@HyukjinKwon Please go ahead since you've already been working on this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22531
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3392/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22531
@srowen, WDYT about we add a rule and exclude them when only they are legitimate cases?
I wrote a rule locally and these below are (almost) all legitimate cases. I was thinking we just explicitly exclude them in the rule (for instance `scalastyle:off ...` ) and we define a rule to enforce as, at least, a reminder to use `Locale.Root`.
```
[error] spark/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesClientApplication.scala:263:6:
[error] spark/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/scheduler/cluster/k8s/ExecutorPodsSnapshot.scala:55:40:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala:62:71:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala:63:17:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala:277:57:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala:279:44:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala:284:22:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:871:38:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:899:38:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:919:31:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:925:46:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:934:42:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:993:76:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:1034:62:
[error] spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveExternalCatalog.scala:1185:31:
[error] spark/resource-managers/mesos/src/main/scala/org/apache/spark/deploy/mesos/MesosClusterDispatcher.scala:63:52:
[error] spark/mllib/src/main/scala/org/apache/spark/ml/feature/Tokenizer.scala:39:5:
[error] spark/mllib/src/main/scala/org/apache/spark/ml/feature/Tokenizer.scala:143:43:
[error] spark/mllib/src/main/scala/org/apache/spark/ml/feature/StopWordsRemover.scala:121:51:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/higherOrderFunctions.scala:76:20:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala:65:67:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala:66:69:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/NumberConverter.scala:96:26:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/ParseDriver.scala:182:18:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:666:30:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:333:53:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:336:38:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:352:53:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:355:38:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:1392:35:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala:1395:40:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/rules/RuleExecutor.scala:147:66:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/util/SchemaUtils.scala:80:77:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:181:72:
[error] spark/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:978:45:
[error] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/InsertIntoHadoopFsRelationCommand.scala:98:55:
[error] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVDataSource.scala:89:56:
[error] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVDataSource.scala:98:62:
[error] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVDataSource.scala:156:39:
[error] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/csv/CSVDataSource.scala:157:39:
[error] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/PartitioningUtils.scala:132:77:
[error] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/PartitioningUtils.scala:327:59:
[error] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/WatermarkTracker.scala:39:14:
[error] spark/sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/SymmetricHashJoinStateManager.scala:266:61:
[error] spark/core/src/main/scala/org/apache/spark/metrics/sink/StatsdSink.scala:55:79:
[error] spark/core/src/main/scala/org/apache/spark/util/Utils.scala:2739:34:
[error] spark/core/src/main/scala/org/apache/spark/rdd/OrderedRDDFunctions.scala:38:53:
[error] spark/core/src/main/scala/org/apache/spark/rdd/OrderedRDDFunctions.scala:38:75:
```
Do you think it's too much or we should add it? I slightly prefer this way but want to listen to your thought before I go and open a PR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/22531
Merged to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22531
**[Test build #96490 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96490/testReport)** for PR 22531 at commit [`d138427`](https://github.com/apache/spark/commit/d138427f35d4980b263dfef21b8810fe455443ca).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on the issue:
https://github.com/apache/spark/pull/22531
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22531
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22531: [SPARK-25415][SQL][FOLLOW-UP] Add Locale.ROOT when toUpp...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22531
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org