[GitHub] [spark] maropu commented on a change in pull request #29506: [SPARK-32672][SQL]Fix data corruption in boolean bit set compression

[GitBox <gi...@apache.org>]

maropu commented on a change in pull request #29506:
URL: https://github.com/apache/spark/pull/29506#discussion_r474739486



##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/columnar/compression/BooleanBitSetSuite.scala
##########
@@ -156,4 +156,32 @@ class BooleanBitSetSuite extends SparkFunSuite {
   test(s"$BooleanBitSet: multiple words and 1 more bit for decompression()") {
     skeletonForDecompress(BITS_PER_LONG * 2 + 1)
   }
+
+  def makeNullBooleanRow(): InternalRow = {
+    val row = new GenericInternalRow(1)
+    row.setNullAt(0)
+    row
+  }
+
+  test(s"$BooleanBitSet: Only nulls for decompression()") {
+    val builder = TestCompressibleColumnBuilder(new NoopColumnStats, BOOLEAN, BooleanBitSet)
+    val numRows = 10
+
+    val rows = Seq.fill[InternalRow](numRows)(makeNullBooleanRow())

Review comment:
       nit:
   ```
       val rows = Seq.fill[InternalRow](numRows) {
         val row = new GenericInternalRow(1)
         row.setNullAt(0)
         row
       }
   ```

##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/compression/compressionSchemes.scala
##########
@@ -318,7 +318,7 @@ private[columnar] case object RunLengthEncoding extends CompressionScheme {
       var valueCountLocal = 0
       var currentValueLocal: Long = 0
 
-      while (valueCountLocal < runLocal || (pos < capacity)) {
+      while (pos < capacity) {

Review comment:
       Could we have `assert(valueCountLocal < runLocal)` here?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org