Repository: segmentio/parquet-go (go)

Issue:
Infinite loop in repeatedPageValues.ReadValues
Run this test to reproduce:
```
func TestLoop(t *testing.T) {
	type testType struct {
		ListOfLists [][]int
	}
	instance := testType{ListOfLists: [][]int{{1, 3, 5}, nil, {2, 4, 6}}}
	buf := parquet.NewGenericBuffer[testType]()
	_, err := buf.Write([]testType{instance})
	require.NoError(t, err)

	rg := parquet.RowGroup(buf)
	chunks := rg.ColumnChunks()
	require.Equal(t, 1, len(chunks))
	pages := chunks[0].Pages()
	p, err := pages.ReadPage()
	require.NoError(t, err)

	vals := make([]parquet.Value, p.NumValues())
	_, _ = p.Values().ReadValues(vals)
}
```

It looks like the infinite loop is caused because this check always evaluates to false:
https://github.com/segmentio/parquet-go/blob/b03c0471ebb0eadd7fc54f41fccd58dafca80eee/page_values.go#L39

As soon as the `nil` is removed from the `testType` instantiation, the test completes successfully.

Pull request:
Panic on schemas containing nested lists without a list tag
Fixes #327 

Patch:
diff --git a/README.md b/README.md
index 6b578ee..ad1b53b 100644
--- a/README.md
+++ b/README.md
@@ -402,6 +402,7 @@ mechanisms to read column values:
 
 ```go
 pages := column.Pages()
+defer checkErr(pages.Close())
 
 for {
     p, err := pages.ReadPage()
diff --git a/column_buffer_go18.go b/column_buffer_go18.go
index 3f06928..c500392 100644
--- a/column_buffer_go18.go
+++ b/column_buffer_go18.go
@@ -67,7 +67,7 @@ func writeRowsFuncOf(t reflect.Type, schema *Schema, path columnPath) writeRowsF
 		return writeRowsFuncOfMap(t, schema, path)
 	}
 
-	panic("cannot convert Go values of type " + t.String() + " to parquet value")
+	panic("cannot convert Go values of type " + typeNameOf(t) + " to parquet value")
 }
 
 func writeRowsFuncOfRequired(t reflect.Type, schema *Schema, path columnPath) writeRowsFunc {
diff --git a/config.go b/config.go
index 6b48f8e..ebd5384 100644
--- a/config.go
+++ b/config.go
@@ -2,13 +2,14 @@ package parquet
 
 import (
 	"fmt"
+	"runtime/debug"
 	"strings"
+	"sync"
 
 	"github.com/segmentio/parquet-go/compress"
 )
 
 const (
-	DefaultCreatedBy            = "github.com/segmentio/parquet-go"
 	DefaultColumnIndexSizeLimit = 16
 	DefaultColumnBufferCapacity = 16 * 1024
 	DefaultPageBufferSize       = 256 * 1024
@@ -19,6 +20,50 @@ const (
 	DefaultSkipBloomFilters     = false
 )
 
+const (
+	parquetGoModulePath = "github.com/segmentio/parquet-go"
+)
+
+var (
+	defaultCreatedByInfo string
+	defaultCreatedByOnce sync.Once
+)
+
+func defaultCreatedBy() string {
+	defaultCreatedByOnce.Do(func() {
+		createdBy := parquetGoModulePath
+		build, ok := debug.ReadBuildInfo()
+		if ok {
+			for _, mod := range build.Deps {
+				if mod.Replace == nil && mod.Path == parquetGoModulePath {
+					semver, _, buildsha := parseModuleVersion(mod.Version)
+					createdBy += " version " + semver + " (build " + buildsha + ")"
+					break
+				}
+			}
+		}
+		defaultCreatedByInfo = createdBy
+	})
+	return defaultCreatedByInfo
+}
+
+func parseModuleVersion(version string) (semver, datetime, buildsha string) {
+	semver, version = splitModuleVersion(version)
+	datetime, version = splitModuleVersion(version)
+	buildsha, _ = splitModuleVersion(version)
+	semver = strings.TrimPrefix(semver, "v")
+	return
+}
+
+func splitModuleVersion(s string) (head, tail string) {
+	if i := strings.IndexByte(s, '-'); i < 0 {
+		head = s
+	} else {
+		head, tail = s[:i], s[i+1:]
+	}
+	return
+}
+
 // The FileConfig type carries configuration options for parquet files.
 //
 // FileConfig implements the FileOption interface so it can be used directly
@@ -150,7 +195,7 @@ type WriterConfig struct {
 // default writer configuration.
 func DefaultWriterConfig() *WriterConfig {
 	return &WriterConfig{
-		CreatedBy:            DefaultCreatedBy,
+		CreatedBy:            defaultCreatedBy(),
 		ColumnPageBuffers:    &defaultPageBufferPool,
 		ColumnIndexSizeLimit: DefaultColumnIndexSizeLimit,
 		PageBufferSize:       DefaultPageBufferSize,
diff --git a/null.go b/null.go
index ab0f752..9cb2277 100644
--- a/null.go
+++ b/null.go
@@ -97,7 +97,7 @@ func nullIndexFuncOf(t reflect.Type) nullIndexFunc {
 		return nullIndexStruct
 	}
 
-	panic("cannot convert Go values of type " + t.String() + " to parquet value")
+	panic("cannot convert Go values of type " + typeNameOf(t) + " to parquet value")
 }
 
 func nullIndexFuncOfByteArray(n int) nullIndexFunc {
diff --git a/page_values.go b/page_values.go
index 85bca61..6cd0d5c 100644
--- a/page_values.go
+++ b/page_values.go
@@ -68,7 +68,13 @@ func (r *repeatedPageValues) ReadValues(values []Value) (n int, err error) {
 	repetitionLevels := r.page.repetitionLevels
 	columnIndex := ^int16(r.page.Column())
 
+	// While we haven't exceeded the output buffer and we haven't exceeded the page size.
 	for n < len(values) && r.offset < len(definitionLevels) {
+
+		// While we haven't exceeded the output buffer and we haven't exceeded the
+		// page size AND the current element's definitionLevel is not the
+		// maxDefinitionLevel (this is a null value), Create the zero values to be
+		// returned in this run.
 		for n < len(values) && r.offset < len(definitionLevels) && definitionLevels[r.offset] != maxDefinitionLevel {
 			values[n] = Value{
 				repetitionLevel: repetitionLevels[r.offset],
@@ -81,11 +87,13 @@ func (r *repeatedPageValues) ReadValues(values []Value) (n int, err error) {
 
 		i := n
 		j := r.offset
+		// Get the length of the run of non-zero values to be copied.
 		for i < len(values) && j < len(definitionLevels) && definitionLevels[j] == maxDefinitionLevel {
 			i++
 			j++
 		}
 
+		// Copy all the non-zero values in this run.
 		if n < i {
 			for j, err = r.values.ReadValues(values[n:i]); j > 0; j-- {
 				values[n].repetitionLevel = repetitionLevels[r.offset]
diff --git a/parquet.go b/parquet.go
index e0cf8fb..69dd5b8 100644
--- a/parquet.go
+++ b/parquet.go
@@ -7,6 +7,8 @@
 // Or see the Parquet documentation: https://parquet.apache.org/docs/
 package parquet
 
+import "reflect"
+
 func atLeastOne(size int) int {
 	return atLeast(size, 1)
 }
@@ -31,3 +33,12 @@ func max(a, b int) int {
 	}
 	return b
 }
+
+func typeNameOf(t reflect.Type) string {
+	s1 := t.String()
+	s2 := t.Kind().String()
+	if s1 == s2 {
+		return s1
+	}
+	return s1 + " (" + s2 + ")"
+}
diff --git a/schema.go b/schema.go
index 894f044..39e0824 100644
--- a/schema.go
+++ b/schema.go
@@ -846,6 +846,13 @@ func makeNodeOf(t reflect.Type, name string, tag []string) Node {
 		node = List(node)
 	}
 
+	if node.Repeated() && !list {
+		elemKind := node.GoType().Elem().Kind()
+		if elemKind == reflect.Slice {
+			panic("unhandled nested slice on parquet schema without list tag")
+		}
+	}
+
 	if optional {
 		node = Optional(node)
 	}


Test patch:
diff --git a/buffer_go18_test.go b/buffer_go18_test.go
index 32a9110..51e08bf 100644
--- a/buffer_go18_test.go
+++ b/buffer_go18_test.go
@@ -165,3 +165,19 @@ func benchmarkGenericBuffer[Row generator[Row]](b *testing.B) {
 		})
 	})
 }
+
+func TestIssue327(t *testing.T) {
+	t.Run("untagged nested lists should panic", func(t *testing.T) {
+		type testType struct {
+			ListOfLists [][]int
+		}
+
+		defer func() {
+			if r := recover(); r == nil {
+				t.Errorf("Nested lists without the list tag should panic")
+			}
+		}()
+
+		_ = parquet.NewGenericBuffer[testType]()
+	})
+}