Repository: parquet-go/parquet-go
Pull request #92 (2023-11-21 03:42:36)

Fix column index reuse
Previously the base column index would return a pointer to the nullpages and nullcounts slices when generating a column index. However if the index was reset and reused these pointers would change the original value of the generated index.

This fixes that by copying the byte slices when generating the index instead of just using the slice pointers 


Patch:
diff --git a/column_index.go b/column_index.go
index dcc2ec1..38914c3 100644
--- a/column_index.go
+++ b/column_index.go
@@ -288,9 +288,13 @@ func (i *baseColumnIndexer) observe(numValues, numNulls int64) {
 }
 
 func (i *baseColumnIndexer) columnIndex(minValues, maxValues [][]byte, minOrder, maxOrder int) format.ColumnIndex {
+	nullPages := make([]bool, len(i.nullPages))
+	copy(nullPages, i.nullPages)
+	nullCounts := make([]int64, len(i.nullCounts))
+	copy(nullCounts, i.nullCounts)
 	return format.ColumnIndex{
-		NullPages:     i.nullPages,
-		NullCounts:    i.nullCounts,
+		NullPages:     nullPages,
+		NullCounts:    nullCounts,
 		MinValues:     minValues,
 		MaxValues:     maxValues,
 		BoundaryOrder: boundaryOrderOf(minOrder, maxOrder),
diff --git a/go.mod b/go.mod
index d7c8e05..bd2c9b0 100644
--- a/go.mod
+++ b/go.mod
@@ -10,8 +10,14 @@ require (
 	github.com/olekukonko/tablewriter v0.0.5
 	github.com/pierrec/lz4/v4 v4.1.18
 	github.com/segmentio/encoding v0.3.6
+	github.com/stretchr/testify v1.8.4
 	golang.org/x/sys v0.10.0
 	google.golang.org/protobuf v1.30.0
 )
 
-require github.com/mattn/go-runewidth v0.0.9 // indirect
+require (
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/mattn/go-runewidth v0.0.9 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
+)
diff --git a/go.sum b/go.sum
index 97f3c36..6b2fb10 100644
--- a/go.sum
+++ b/go.sum
@@ -1,5 +1,7 @@
 github.com/andybalholm/brotli v1.0.5 h1:8uQZIdzKmjc/iuPu7O2ioW48L81FgatrcpfFmiq/cCs=
 github.com/andybalholm/brotli v1.0.5/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig=
+github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
 github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
@@ -15,9 +17,13 @@ github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N
 github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY=
 github.com/pierrec/lz4/v4 v4.1.18 h1:xaKrnTkyoqfh1YItXl56+6KJNVYWlEEPuAQW9xsplYQ=
 github.com/pierrec/lz4/v4 v4.1.18/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg=
 github.com/segmentio/encoding v0.3.6 h1:E6lVLyDPseWEulBmCmAKPanDd3jiyGDo5gMcugCRwZQ=
 github.com/segmentio/encoding v0.3.6/go.mod h1:n0JeuIqEQrQoPDGsjo8UNd1iA0U8d8+oHAA4E3G3OxM=
+github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
+github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 golang.org/x/sys v0.0.0-20211110154304-99a53858aa08/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA=
 golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
@@ -26,3 +32,7 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T
 google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
 google.golang.org/protobuf v1.30.0 h1:kPPoIgf3TsEvrm0PFe15JQ+570QVxYzEvvHqChK+cng=
 google.golang.org/protobuf v1.30.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=


Test patch:
diff --git a/column_index_test.go b/column_index_test.go
index 10ce032..3ecf08f 100644
--- a/column_index_test.go
+++ b/column_index_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/parquet-go/parquet-go"
+	"github.com/stretchr/testify/require"
 )
 
 func TestBinaryColumnIndexMinMax(t *testing.T) {
@@ -58,3 +59,38 @@ func TestBinaryColumnIndexMinMax(t *testing.T) {
 		}
 	}
 }
+
+func Test_ColumnIndexReuse(t *testing.T) {
+	min := "a"
+	max := "z"
+	indexer := parquet.ByteArrayType.NewColumnIndexer(16)
+	indexer.IndexPage(100, 0,
+		parquet.ValueOf(min),
+		parquet.ValueOf(max),
+	)
+	before := indexer.ColumnIndex()
+	require.Equal(t, 1, len(before.NullPages))
+	require.False(t, before.NullPages[0])
+
+	// Reset the indexer. Should be safe for reuse.
+	indexer.Reset()
+
+	// Index two pages that are both nul pages, expect the previous index to not have changed.
+	indexer.IndexPage(100, 100,
+		parquet.ValueOf(min),
+		parquet.ValueOf(max),
+	)
+	indexer.IndexPage(10, 10,
+		parquet.ValueOf(min),
+		parquet.ValueOf(max),
+	)
+	after := indexer.ColumnIndex()
+
+	require.Equal(t, 2, len(after.NullPages))
+	require.True(t, after.NullPages[0])
+	require.True(t, after.NullPages[1])
+
+	// Validate null pages of the previous index.
+	require.Equal(t, 1, len(before.NullPages))
+	require.False(t, before.NullPages[0])
+}