Repository: segmentio/parquet-go (go)

Issue:
Library sometimes flips boolean values
Somewhat regularly parquet-go will flip boolean values. The test at the bottom reproduces this issue.

Some things I've noticed:
- the issue occurs even if i test with the `purego` tag
- if i don't call `w.Flush()` in the write loop and allow `w.Close()` to flush the issue does not occur
- if i don't randomize the bool set to true the issue does not occur

```
type Bools struct {
	A bool
	B bool
}

func TestBools(t *testing.T) {
	f, err := os.CreateTemp(t.TempDir(), "data.parquet")
	if err != nil {
		t.Fatal(err)
	}
	defer f.Close()
	w := parquet.NewGenericWriter[*Bools](f)

	// write some data
	for i := 0; i < 100; i++ {
		// set one random bool true
		st := &Bools{}
		n := rand.Intn(2)
		switch n {
		case 0:
			st.A = true
		case 1:
			st.B = true
		}
		w.Write([]*Bools{st})
		if i%10 == 0 {
			w.Flush()
		}
	}
	w.Close()

	stat, err := f.Stat()
	if err != nil {
		t.Fatal(err)
	}

	pf, err := parquet.OpenFile(f, stat.Size())
	if err != nil {
		t.Fatal(err)
	}

	r := parquet.NewGenericReader[*Bools](pf)
	for {
		sts := make([]*Bools, 1)
		_, err := r.Read(sts)
		st := sts[0]
		if st != nil {
			if st.A && st.B {
				t.Fatal("both true")
			}
		}
		if err == io.EOF {
			break
		}
		if err != nil {
			t.Fatal(err)
		}
	}
}
```

Pull request:
Fix #501
This PR aims to fix #501. I'm not 100% confident in this fix, but based on my understanding of the code in question this is valid.

The issue appears to be this line here:

https://github.com/segmentio/parquet-go/blob/main/column_buffer.go#L851

The code builds a bitmask and assigns the value to `b` to set relevant booleans in the current bit. Then it `|=`s the bitmask against the current bit. Interestingly using `|=` guarantees that this path can never drop an existing bit flag to 0 and whatever happened to be leftover in the buffer persists.

Currently running the Tempo test suite against this.

Patch:
diff --git a/column_buffer.go b/column_buffer.go
index 35d909c..53e6cb3 100644
--- a/column_buffer.go
+++ b/column_buffer.go
@@ -848,7 +848,7 @@ func (col *booleanColumnBuffer) writeValues(rows sparse.Array, _ columnLevels) {
 			}
 			x := uint(col.numValues) / 8
 			y := uint(col.numValues) % 8
-			col.bits[x] |= (b << y) | (col.bits[x] & ^(0xFF << y))
+			col.bits[x] = (b << y) | (col.bits[x] & ^(0xFF << y))
 			col.numValues += int32(i)
 		}
 


Test patch:
diff --git a/column_buffer_test.go b/column_buffer_test.go
index 351369d..5c0f6a3 100644
--- a/column_buffer_test.go
+++ b/column_buffer_test.go
@@ -1,6 +1,8 @@
 package parquet
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestBroadcastValueInt32(t *testing.T) {
 	buf := make([]int32, 123)
@@ -39,3 +41,37 @@ func BenchmarkBroadcastRangeInt32(b *testing.B) {
 	}
 	b.SetBytes(4 * int64(len(buf)))
 }
+
+// https://github.com/segmentio/parquet-go/issues/501
+func TestIssue501(t *testing.T) {
+	col := newBooleanColumnBuffer(BooleanType, 0, 2055208)
+
+	// write all trues and then flush the buffer
+	_, err := col.WriteBooleans([]bool{true, true, true, true, true, true, true, true})
+	if err != nil {
+		t.Fatal(err)
+	}
+	col.Reset()
+
+	// write a single false, we are trying to trip a certain line of code in WriteBooleans
+	_, err = col.WriteBooleans([]bool{false})
+	if err != nil {
+		t.Fatal(err)
+	}
+	// now write 7 booleans at once, this will cause WriteBooleans to attempt its "alignment" logic
+	_, err = col.WriteBooleans([]bool{false, false, false, false, false, false, false})
+	if err != nil {
+		panic(err)
+	}
+
+	for i := 0; i < 8; i++ {
+		read := make([]Value, 1)
+		_, err = col.ReadValuesAt(read, int64(i))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if read[0].Boolean() {
+			t.Fatalf("expected false at index %d", i)
+		}
+	}
+}