Skip to content

[SPARK-25108][SQL] Fix the show method to display the wide character alignment problem #22048

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
xuejianbest wants to merge 11 commits into from
Closed

[SPARK-25108][SQL] Fix the show method to display the wide character alignment problem #22048

Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1b9b2e7
Fix the show method to display the wide character alignment problem
xuejianbest Aug 9, 2018
9aec12f
Merge branch 'master' into master
xuejianbest Aug 9, 2018
906c0ba
Modifying regular expressions for matching narrow characters
xuejianbest Aug 10, 2018
da37d2e
Fix the show method to display full width characters.
xuejianbest Aug 28, 2018
8737671
Write a utility method to get the number of half width of string
xuejianbest Aug 29, 2018
697ac04
Move the stringHalfWidth method into util.Utils
xuejianbest Aug 29, 2018
3d65e6b
Formatted code and non-functional changes
xuejianbest Aug 30, 2018
363de6b
Add tests in DatasetSuite
xuejianbest Aug 30, 2018
3649de5
Modified variable name in DatasetSuite
xuejianbest Aug 30, 2018
45ac272
Adding scalastyle:off nonascii in code
xuejianbest Aug 31, 2018
52acfd5
Fix the show method to display the full width character alignment pro…
xuejianbest Sep 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,23 +294,24 @@ class Dataset[T] private[sql](
// We set a minimum column width at '3'
val minimumColWidth = 3

val regex = """[^\x00-\u2e39]""".r
Copy link
Member

@srowen srowen Aug 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could use a comment and slightly better name for the variable? I also wonder if a regex is a little bit slow for scanning every character.

However it's not clear this definition is accurate enough. According to things like https://stackoverflow.com/questions/13505075/analyzing-full-width-or-half-width-character-in-java we're really looking for the concept of "fullwidth" East Asian characters. The answer there provides a somewhat more precise definition, though others say you need something like icu4j for a proper definition.

Maybe at least adopt the alternative proposed in the SO answer? It's not necessary to be perfect here, as it's a cosmetic issue.

Copy link
Member

@kiszk kiszk Aug 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @srowen's comment. To make it clear, could you please add tests for more characters?

if (!vertical) {
// Initialise the width of each column to a minimum value
val colWidths = Array.fill(numCols)(minimumColWidth)

// Compute the width of each column
for (row <- rows) {
for ((cell, i) <- row.zipWithIndex) {
colWidths(i) = math.max(colWidths(i), cell.length)
colWidths(i) = math.max(colWidths(i), cell.length + regex.findAllIn(cell).size)
}
}

val paddedRows = rows.map { row =>
row.zipWithIndex.map { case (cell, i) =>
if (truncate > 0) {
StringUtils.leftPad(cell, colWidths(i))
StringUtils.leftPad(cell, colWidths(i) - regex.findAllIn(cell).size)
} else {
StringUtils.rightPad(cell, colWidths(i))
StringUtils.rightPad(cell, colWidths(i) - regex.findAllIn(cell).size)
}
}
}
Expand All @@ -332,12 +333,10 @@ class Dataset[T] private[sql](

// Compute the width of field name and data columns
val fieldNameColWidth = fieldNames.foldLeft(minimumColWidth) { case (curMax, fieldName) =>
math.max(curMax, fieldName.length)
math.max(curMax, fieldName.length + regex.findAllIn(fieldName).size)
}
val dataColWidth = dataRows.foldLeft(minimumColWidth) { case (curMax, row) =>
math.max(curMax, row.map(_.length).reduceLeftOption[Int] { case (cellMax, cell) =>
math.max(cellMax, cell)
}.getOrElse(0))
math.max(curMax, row.map(cell => cell.length + regex.findAllIn(cell).size).max)
}

dataRows.zipWithIndex.foreach { case (row, i) =>
Expand All @@ -346,8 +345,8 @@ class Dataset[T] private[sql](
s"-RECORD $i", fieldNameColWidth + dataColWidth + 5, "-")
sb.append(rowHeader).append("\n")
row.zipWithIndex.map { case (cell, j) =>
val fieldName = StringUtils.rightPad(fieldNames(j), fieldNameColWidth)
val data = StringUtils.rightPad(cell, dataColWidth)
val fieldName = StringUtils.rightPad(fieldNames(j), fieldNameColWidth - regex.findAllIn(fieldNames(j)).size)
val data = StringUtils.rightPad(cell, dataColWidth - regex.findAllIn(cell).size)
s" $fieldName | $data "
}.addString(sb, "", "\n", "\n")
}
Expand Down