Skip to content

Conversation

@maver1ck
Copy link
Contributor

@maver1ck maver1ck commented Jul 11, 2016

What changes were proposed in this pull request?

Spark SQL UI display numbers greater than 1000 with u00A0 as grouping separator.
Problem exists when server locale has no-breaking space as separator. (for example pl_PL)
This patch turns off grouping and remove this separator.

The problem starts with this PR.
https://github.com/apache/spark/pull/12425/files#diff-803f475b01acfae1c5c96807c2ea9ddcR125

How was this patch tested?

Manual UI tests. Screenshot attached.

image

@srowen
Copy link
Member

srowen commented Jul 12, 2016

LGTM. Actually all other instances of NumberFormat in the project omit grouping separators, for various reasons.

@maver1ck
Copy link
Contributor Author

Can we test this ?

@srowen
Copy link
Member

srowen commented Jul 12, 2016

Jenkins add to whitelist

@srowen
Copy link
Member

srowen commented Jul 12, 2016

Jenkins test this please

@SparkQA
Copy link

SparkQA commented Jul 12, 2016

Test build #62163 has finished for PR 14142 at commit fef15ce.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maver1ck
Copy link
Contributor Author

Merging ?

@srowen
Copy link
Member

srowen commented Jul 13, 2016

Yes, no need to ping inside of a day or two. We will often leave it open at least that long in case anyone has comments.

@srowen
Copy link
Member

srowen commented Jul 13, 2016

merged to master/2.0

@asfgit asfgit closed this in 83879eb Jul 13, 2016
asfgit pushed a commit that referenced this pull request Jul 13, 2016
## What changes were proposed in this pull request?

Spark SQL UI display numbers greater than 1000 with u00A0 as grouping separator.
Problem exists when server locale has no-breaking space as separator. (for example pl_PL)
This patch turns off grouping and remove this separator.

The problem starts with this PR.
https://github.com/apache/spark/pull/12425/files#diff-803f475b01acfae1c5c96807c2ea9ddcR125

## How was this patch tested?

Manual UI tests. Screenshot attached.

![image](https://cloud.githubusercontent.com/assets/4006010/16749556/5cb5a372-47cb-11e6-9a95-67fd3f9d1c71.png)

Author: Maciej Brynski <[email protected]>

Closes #14142 from maver1ck/master.

(cherry picked from commit 83879eb)
Signed-off-by: Sean Owen <[email protected]>
asfgit pushed a commit that referenced this pull request Sep 19, 2016
## What changes were proposed in this pull request?

Currently, the SQL metrics looks like `number of rows: 111111111111`, it's very hard to read how large the number is. So a separator was added by #12425, but removed by #14142, because the separator is weird in some locales (for example, pl_PL), this PR will add that back, but always use "," as the separator, since the SQL UI are all in English.

## How was this patch tested?

Existing tests.
![metrics](https://cloud.githubusercontent.com/assets/40902/14573908/21ad2f00-030d-11e6-9e2c-c544f30039ea.png)

Author: Davies Liu <[email protected]>

Closes #15106 from davies/metric_sep.

(cherry picked from commit e063206)
Signed-off-by: Davies Liu <[email protected]>
asfgit pushed a commit that referenced this pull request Sep 19, 2016
## What changes were proposed in this pull request?

Currently, the SQL metrics looks like `number of rows: 111111111111`, it's very hard to read how large the number is. So a separator was added by #12425, but removed by #14142, because the separator is weird in some locales (for example, pl_PL), this PR will add that back, but always use "," as the separator, since the SQL UI are all in English.

## How was this patch tested?

Existing tests.
![metrics](https://cloud.githubusercontent.com/assets/40902/14573908/21ad2f00-030d-11e6-9e2c-c544f30039ea.png)

Author: Davies Liu <[email protected]>

Closes #15106 from davies/metric_sep.
wgtmac pushed a commit to wgtmac/spark that referenced this pull request Sep 19, 2016
## What changes were proposed in this pull request?

Currently, the SQL metrics looks like `number of rows: 111111111111`, it's very hard to read how large the number is. So a separator was added by apache#12425, but removed by apache#14142, because the separator is weird in some locales (for example, pl_PL), this PR will add that back, but always use "," as the separator, since the SQL UI are all in English.

## How was this patch tested?

Existing tests.
![metrics](https://cloud.githubusercontent.com/assets/40902/14573908/21ad2f00-030d-11e6-9e2c-c544f30039ea.png)

Author: Davies Liu <[email protected]>

Closes apache#15106 from davies/metric_sep.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants