Skip to content

[SPARK-16439] [SQL] bring back the separator in SQL UI - #15106

Closed
davies wants to merge 1 commit into
apache:masterfrom
davies:metric_sep
Closed

[SPARK-16439] [SQL] bring back the separator in SQL UI#15106
davies wants to merge 1 commit into
apache:masterfrom
davies:metric_sep

Conversation

@davies

Copy link
Copy Markdown
Contributor

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

@davies

Copy link
Copy Markdown
ContributorAuthor

cc @maver1ck@srowen

@SparkQA

Copy link
Copy Markdown

Test build #65412 has finished for PR 15106 at commit 11a989e.

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

@srowen

Copy link
Copy Markdown
Member

The question isn't the language of the UI, but of the host OS. I think this just causes the previous problem again, and that seems more important. I don't think we can make this change as is.

@maver1ck

Copy link
Copy Markdown
Contributor

I think this patch could actually work.
Number format is executed on the server side.
I did some tests and it looks good.

@davies

Copy link
Copy Markdown
ContributorAuthor

@srowen The previous problem is caused by using the default locale of host to format the numbers, that sounds perfect by caused some problems. So we fallback to only use English as the locale, it will works well everywhere.

@davies

Copy link
Copy Markdown
ContributorAuthor

@srowen Could you left an lgtm here?

@srowen

Copy link
Copy Markdown
Member

If @maver1ck is OK with it I am, sure.

@maver1ck

Copy link
Copy Markdown
Contributor

LGTM.

@davies

Copy link
Copy Markdown
ContributorAuthor

Merging this into master and 2.0, thanks!

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 <davies@databricks.com>
Closes#15106 from davies/metric_sep.
(cherry picked from commit e063206)
Signed-off-by: Davies Liu <davies.liu@gmail.com>
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 <davies@databricks.com>
Closesapache#15106 from davies/metric_sep.
Sign up for freeto 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.

4 participants

@davies@SparkQA@srowen@maver1ck