Skip to content
Closed
Changes from 4 commits
Commits
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
18 changes: 9 additions & 9 deletions sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala
Original file line number Diff line number Diff line change
Expand Up @@ -294,23 +294,25 @@ class Dataset[T] private[sql](
// We set a minimum column width at '3'
val minimumColWidth = 3

//Regular expression matching full width characters
Copy link
Member

Choose a reason for hiding this comment

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

Very small nit that might fail scalastyle -- space after comment slashes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, thanks.

val fullWidthRegex = """[\u1100-\u115F\u2E80-\uA4CF\uAC00-\uD7A3\uF900-\uFAFF\uFE10-\uFE19\uFE30-\uFE6F\uFF00-\uFF60\uFFE0-\uFFE6]""".r
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if detecting this using a regex for each character is slow? probably not meaningfully enough to care about, but I wonder.

Copy link
Contributor Author

@xuejianbest xuejianbest Aug 29, 2018

Choose a reason for hiding this comment

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

I generated 1000 strings, each consisting of 1000 characters with a random unicode of 0x0000-0xFFFF. (a total of 1 million characters.)
Then use this regular expression to find the full width character of these strings.
I tested 100 rounds and then averaged.
It takes 49 milliseconds to complete matching all 1000 strings.

Copy link
Member

Choose a reason for hiding this comment

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

OK, not worth optimizing now.

Copy link
Member

Choose a reason for hiding this comment

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

This line goes over the limit, 100.

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 + fullWidthRegex.findAllIn(cell).size)
Copy link
Member

Choose a reason for hiding this comment

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

What about a utility method for the x.length + fullWidthRegex.findAllIn(x).size expression that recurs here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I committed a new version. See if this is appropriate please ?
@srowen

}
}

val paddedRows = rows.map { row =>
row.zipWithIndex.map { case (cell, i) =>
if (truncate > 0) {
StringUtils.leftPad(cell, colWidths(i))
StringUtils.leftPad(cell, colWidths(i) - fullWidthRegex.findAllIn(cell).size)
} else {
StringUtils.rightPad(cell, colWidths(i))
StringUtils.rightPad(cell, colWidths(i) - fullWidthRegex.findAllIn(cell).size)
}
}
}
Expand All @@ -332,12 +334,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 + fullWidthRegex.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 + fullWidthRegex.findAllIn(cell).size).max)
}

dataRows.zipWithIndex.foreach { case (row, i) =>
Expand All @@ -346,8 +346,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 - fullWidthRegex.findAllIn(fieldNames(j)).size)
val data = StringUtils.rightPad(cell, dataColWidth - fullWidthRegex.findAllIn(cell).size)
s" $fieldName | $data "
}.addString(sb, "", "\n", "\n")
}
Expand Down