Repository: parquet-go/parquet-go (go)

Issue:
Merging row groups throws panic when rows from one group are empty
Merging multiple sorted row groups will throw a panic when one of the groups returns empty rows during `ReadRows`, even if `n` is correctly returned as 0. The reason why we sometimes return empty rows is because we apply filtering during the merge process, and occasionally no rows will match the filter. I think this only happens when the entire row group is empty.

Here is a PR reproducing the issue: https://github.com/parquet-go/parquet-go/pull/67.

Pull request:
fix merging empty row groups
fixes #66
closes #67

Patch:
diff --git a/merge.go b/merge.go
index 7a486a5..cc8df4a 100644
--- a/merge.go
+++ b/merge.go
@@ -460,6 +460,10 @@ func (m *mergeBuffer) left() bool {
 func (m *mergeBuffer) read() (n int64) {
 	for n < int64(len(m.copy)) && m.Len() != 0 {
 		r := m.buffer[:m.len][0]
+		if len(r) == 0 {
+			heap.Pop(m)
+			continue
+		}
 		m.copy[n] = append(m.copy[n][:0], r[m.head[0]]...)
 		m.head[0]++
 		n++


Test patch:
diff --git a/merge_test.go b/merge_test.go
index 9181924..f484ccc 100644
--- a/merge_test.go
+++ b/merge_test.go
@@ -135,7 +135,53 @@ func TestMergeRowGroups(t *testing.T) {
 				Person{FirstName: "Han", LastName: "Solo"},
 			),
 		},
+		{
+			scenario: "reproduce issue #66, merging rows with an empty row group",
+			options: []parquet.RowGroupOption{
+				parquet.SortingRowGroupConfig(
+					parquet.SortingColumns(
+						parquet.Ascending("LastName"),
+					),
+				),
+			},
+			input: []parquet.RowGroup{
+				sortedRowGroup(
+					[]parquet.RowGroupOption{
+						parquet.SortingRowGroupConfig(
+							parquet.SortingColumns(
+								parquet.Ascending("LastName"),
+							),
+						),
+					},
+					Person{FirstName: "Han", LastName: "Solo"},
+				),
 
+				sortedRowGroup(
+					[]parquet.RowGroupOption{
+						parquet.SchemaOf(Person{}),
+						parquet.SortingRowGroupConfig(
+							parquet.SortingColumns(
+								parquet.Ascending("LastName"),
+							),
+						),
+					},
+				),
+				sortedRowGroup(
+					[]parquet.RowGroupOption{
+						parquet.SortingRowGroupConfig(
+							parquet.SortingColumns(
+								parquet.Ascending("LastName"),
+							),
+						),
+					},
+					Person{FirstName: "Obiwan", LastName: "Kenobi"},
+				),
+			},
+			output: sortedRowGroup(nil,
+				Person{FirstName: "Obiwan", LastName: "Kenobi"},
+				Person{FirstName: "Han", LastName: "Solo"},
+			),
+		},
 		{
 			scenario: "row groups sorted by descending last name",
 			options: []parquet.RowGroupOption{
diff --git a/row_group_test.go b/row_group_test.go
index bb35471..44c84e5 100644
--- a/row_group_test.go
+++ b/row_group_test.go
@@ -120,7 +120,18 @@ func fileRowGroup(rowGroup parquet.RowGroup) parquet.RowGroup {
 	if err != nil {
 		panic(err)
 	}
-	return f.RowGroups()[0]
+	g := f.RowGroups()
+	if len(g) > 0 {
+		return g[0]
+	}
+	// There is a test checking for a panic when merging empty row groups. One of
+	// the input is an empty row group which leads to this path.
+	//
+	// It is unnecessary to also return an empty row group here because the
+	// behavior is triggered by custom row group implementation.
+	//
+	// buffer  scenario check  should be sufficient to cover for the issue.
+	return nil
 }
 
 func TestWriteRowGroupClosesRows(t *testing.T) {