Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/20636#discussion_r206638094
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/codegen/BufferHolderSparkSubmitSuite.scala
---
@@ -55,22 +55,30 @@ object BufferHolderSparkSubmitSuite {
val ARRAY_MAX = ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH
- val holder = new BufferHolder(new UnsafeRow(1000))
+ val unsafeRow = new UnsafeRow(1000)
+ val holder = new BufferHolder(unsafeRow)
holder.reset()
- holder.grow(roundToWord(ARRAY_MAX / 2))
- holder.reset()
- holder.grow(roundToWord(ARRAY_MAX / 2 + 8))
+ // while to reuse a buffer may happen, this test checks whether the
buffer can be grown
+ holder.grow(ARRAY_MAX / 2)
+ assert(unsafeRow.getSizeInBytes % 8 == 0)
- holder.reset()
- holder.grow(roundToWord(Integer.MAX_VALUE / 2))
+ holder.grow(ARRAY_MAX / 2 + 7)
+ assert(unsafeRow.getSizeInBytes % 8 == 0)
- holder.reset()
- holder.grow(roundToWord(Integer.MAX_VALUE))
- }
+ holder.grow(Integer.MAX_VALUE / 2)
+ assert(unsafeRow.getSizeInBytes % 8 == 0)
+
+ holder.grow(ARRAY_MAX - holder.totalSize())
+ assert(unsafeRow.getSizeInBytes % 8 == 0)
- private def roundToWord(len: Int): Int = {
- ByteArrayMethods.roundNumberOfBytesToNearestWord(len)
+ try {
+ holder.grow(ARRAY_MAX + 1 - holder.totalSize())
+ assert(false)
+ } catch {
+ case _: UnsupportedOperationException => assert(true)
--- End diff --
Fix the indents here. `assert(true)` is a no-op, so just omit it.
`assert(false)` is less useful than `fail(...message...)`, above. Let an
unexpected `Throwable` just fly out of the method to fail it rather than
swallow it. But do you really just want to use `intercept` here?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
