Fixed numeric merging options (#74)

This PR fixes merging options on numerical columns, the argument was not
passed properly. Also added a few tests and removed mandatory merge
function on record column, to make the API more consistent.

Author: kelindar

.github/workflows/test.yml

@@ -9,7 +9,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go: ["1.18"]
+        go: ["1.19"]
     steps:
       - name: Set up Go ${{ matrix.go }}
         uses: actions/setup-go@v3

README.md

@@ -371,7 +371,7 @@ Now that we have a record implementation, we can create a column for this struct
 ```go
 players.CreateColumn("location", ForRecord(func() *Location {
 	return new(Location)
-}, nil)) // no merging
+}))
 ```
 
 In order to manipulate the record, we can use the appropriate `Record()`, `SetRecord()` methods of the `Row`, similarly to other column types.

codegen/numbers.tpl

@@ -29,7 +29,7 @@ func make{{.Name}}s(opts ...func(*option[{{.Type}}])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 

collection_test.go

@@ -193,7 +193,7 @@ func BenchmarkRecord(b *testing.B) {
 		col := NewCollection()
 		col.CreateColumn("ts", ForRecord(func() *time.Time {
 			return new(time.Time)
-		}, nil))
+		}))
 
 		for i := 0; i < amount; i++ {
 			col.Insert(func(r Row) error {
@@ -247,7 +247,6 @@ func BenchmarkRecord(b *testing.B) {
 			})
 		}
 	})
-
 }
 
 func TestCollection(t *testing.T) {
@@ -802,7 +801,7 @@ func newEmpty(capacity int) *Collection {
 	out.CreateColumn("guild", ForEnum())
 	out.CreateColumn("location", ForRecord(func() *fixtures.Location {
 		return new(fixtures.Location)
-	}, nil))
+	}))
 
 	// index on humans
 	out.CreateIndex("human", "race", func(r Reader) bool {

column.go

@@ -116,7 +116,7 @@ func ForKind(kind reflect.Kind) (Column, error) {
 
 // --------------------------- Generic Options ----------------------------
 
-// optInt represents options for variouos columns.
+// option represents options for variouos columns.
 type option[T any] struct {
 	Merge func(value, delta T) T
 }

column_numbers.go

@@ -29,7 +29,7 @@ func makeInts(opts ...func(*option[int])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 
@@ -78,7 +78,7 @@ func makeInt16s(opts ...func(*option[int16])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 
@@ -127,7 +127,7 @@ func makeInt32s(opts ...func(*option[int32])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 
@@ -176,7 +176,7 @@ func makeInt64s(opts ...func(*option[int64])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 
@@ -225,7 +225,7 @@ func makeUints(opts ...func(*option[uint])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 
@@ -274,7 +274,7 @@ func makeUint16s(opts ...func(*option[uint16])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 
@@ -323,7 +323,7 @@ func makeUint32s(opts ...func(*option[uint32])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 
@@ -372,7 +372,7 @@ func makeUint64s(opts ...func(*option[uint64])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 
@@ -421,7 +421,7 @@ func makeFloat32s(opts ...func(*option[float32])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 
@@ -470,7 +470,7 @@ func makeFloat64s(opts ...func(*option[float64])) Column {
 					fill.Remove(offset)
 				}
 			}
-		},
+		}, opts,
 	)
 }
 

column_numeric.go

@@ -37,7 +37,7 @@ type numericColumn[T simd.Number] struct {
 func makeNumeric[T simd.Number](
 	write func(*commit.Buffer, uint32, T),
 	apply func(*commit.Reader, bitmap.Bitmap, []T, option[T]),
-	opts ...func(*option[T]),
+	opts []func(*option[T]),
 ) *numericColumn[T] {
 	return &numericColumn[T]{
 		chunks: make(chunks[T], 0, 4),

column_record.go

@@ -28,10 +28,10 @@ type columnRecord struct {
 // ForRecord creates a new column that contains a type marshaled into/from binary. It requires
 // a constructor for the type as well as optional merge function. If merge function is
 // set to nil, "overwrite" strategy will be used.
-func ForRecord[T recordType](new func() T, merge func(value, delta T) T) Column {
-	if merge == nil {
-		merge = func(value, delta T) T { return delta }
-	}
+func ForRecord[T recordType](new func() T, opts ...func(*option[T])) Column {
+	mergeFunc := configure(opts, option[T]{
+		Merge: func(value, delta T) T { return delta },
+	}).Merge
 
 	pool := &sync.Pool{
 		New: func() any { return new() },
@@ -52,10 +52,8 @@ func ForRecord[T recordType](new func() T, merge func(value, delta T) T) Column
 			return v
 		}
 
-		// Apply the user-defined merging strategy
-		merged := merge(value, delta)
-
-		// Marshal the value back
+		// Apply the user-defined merging strategy and marshal it back
+		merged := mergeFunc(value, delta)
 		if encoded, err := merged.MarshalBinary(); err == nil {
 			return b2s(&encoded)
 		}

column_test.go

@@ -579,7 +579,7 @@ func TestRecord(t *testing.T) {
 	col := NewCollection()
 	col.CreateColumn("ts", ForRecord(func() *time.Time {
 		return new(time.Time)
-	}, nil))
+	}))
 
 	// Insert the time, it implements binary marshaler
 	idx, _ := col.Insert(func(r Row) error {
@@ -609,7 +609,7 @@ func TestRecord_Errors(t *testing.T) {
 	col.CreateColumn("id", ForInt64())
 	col.CreateColumn("ts", ForRecord(func() *time.Time {
 		return new(time.Time)
-	}, nil))
+	}))
 
 	// Column "xxx" does not exist
 	assert.Panics(t, func() {
@@ -643,7 +643,7 @@ func TestRecordMerge_ErrDecode(t *testing.T) {
 		return mockRecord{
 			errDecode: true,
 		}
-	}, nil))
+	}))
 
 	// Insert the time, it implements binary marshaler
 	idx, _ := col.Insert(func(r Row) error {
@@ -664,7 +664,7 @@ func TestRecordMerge_ErrEncode(t *testing.T) {
 		return mockRecord{
 			errEncode: true,
 		}
-	}, nil))
+	}))
 
 	// Insert the time, it implements binary marshaler
 	idx, _ := col.Insert(func(r Row) error {
@@ -679,6 +679,33 @@ func TestRecordMerge_ErrEncode(t *testing.T) {
 	})
 }
 
+func TestNumberMerge(t *testing.T) {
+	col := NewCollection()
+	col.CreateColumn("age", ForInt32(WithMerge(func(v, d int32) int32 {
+		v -= d
+		return v
+	})))
+
+	// Insert the time, it implements binary marshaler
+	idx, _ := col.Insert(func(r Row) error {
+		r.SetInt32("age", 10)
+		return nil
+	})
+
+	for i := 0; i < 10; i++ {
+		col.QueryAt(idx, func(r Row) error {
+			r.MergeInt32("age", 1)
+			return nil
+		})
+	}
+
+	col.QueryAt(idx, func(r Row) error {
+		age, _ := r.Int32("age")
+		assert.Equal(t, int32(0), age)
+		return nil
+	})
+}
+
 func invoke(any interface{}, name string, args ...interface{}) []reflect.Value {
 	inputs := make([]reflect.Value, len(args))
 	for i := range args {

commit/buffer_test.go

@@ -306,3 +306,17 @@ func TestBufferReadFromFailures(t *testing.T) {
 		assert.Error(t, err)
 	}
 }
+
+func FuzzBufferString(f *testing.F) {
+	f.Add(uint32(1), "test")
+
+	f.Fuzz(func(t *testing.T, i uint32, v string) {
+		buf := NewBuffer(0)
+		buf.PutString(Put, i, v)
+
+		r := NewReader()
+		r.Seek(buf)
+		assert.True(t, r.Next())
+		assert.Equal(t, v, r.String())
+	})
+}