Repository: polarsignals/frostdb
Pull request #675 (2024-01-08 16:49:53)

Return transaction on (*GenericTable[T]).Write
We are wrapping `(*Table).InsertRecord` call but were ignoring the transaction . This commit ensures we match `(*Table).InsertRecord`  return signature.

I also added godoc on the method

asubiotto: I think this is a breaking API change. I would backburner this unless we find a clear need for it cc @thorfour 
gernest: 
> I think this is a breaking API change. I would backburner this unless we find a clear need for it cc @thorfour

Agreed..

While we are on this subject, any reason we don't have tagged releases ?

asubiotto: Nope

Patch:
diff --git a/examples/aggregations/aggregations.go b/examples/aggregations/aggregations.go
index 0187f70..4f73337 100644
--- a/examples/aggregations/aggregations.go
+++ b/examples/aggregations/aggregations.go
@@ -49,7 +49,7 @@ func main() {
 		"state": "Minnesota",
 	}
 
-	_ = table.Write(context.Background(),
+	_, _ = table.Write(context.Background(),
 		WeatherRecord{Day: "Mon", Snowfall: 20, City: montreal},
 		WeatherRecord{Day: "Tue", Snowfall: 00, City: montreal},
 		WeatherRecord{Day: "Wed", Snowfall: 30, City: montreal},
diff --git a/examples/simple/simple.go b/examples/simple/simple.go
index 2bf1eb5..22926da 100644
--- a/examples/simple/simple.go
+++ b/examples/simple/simple.go
@@ -44,7 +44,7 @@ func main() {
 		},
 		Value: 99,
 	}
-	_ = table.Write(context.Background(), frederic, thor)
+	_, _ = table.Write(context.Background(), frederic, thor)
 
 	// Now we can insert rows that have middle names into our dynamic column
 	matthias := Simple{
@@ -55,7 +55,7 @@ func main() {
 		},
 		Value: 101,
 	}
-	_ = table.Write(context.Background(), matthias)
+	_, _ = table.Write(context.Background(), matthias)
 
 	// Create a new query engine to retrieve data and print the results
 	engine := query.NewEngine(memory.DefaultAllocator, database.TableProvider())
diff --git a/table.go b/table.go
index 0eacdf7..80c00ab 100644
--- a/table.go
+++ b/table.go
@@ -237,15 +237,15 @@ func (t *GenericTable[T]) Release() {
 	t.build.Release()
 }
 
-func (t *GenericTable[T]) Write(ctx context.Context, values ...T) error {
+// Write builds arrow.Record directly from values and calls (*Table).InsertRecord.
+func (t *GenericTable[T]) Write(ctx context.Context, values ...T) (uint64, error) {
 	t.mu.Lock()
 	defer t.mu.Unlock()
 	err := t.build.Append(values...)
 	if err != nil {
-		return err
+		return 0, err
 	}
-	_, err = t.InsertRecord(ctx, t.build.NewRecord())
-	return err
+	return t.InsertRecord(ctx, t.build.NewRecord())
 }
 
 func NewGenericTable[T any](db *DB, name string, mem memory.Allocator, options ...TableOption) (*GenericTable[T], error) {


Test patch:
diff --git a/aggregate_test.go b/aggregate_test.go
index d4b6588..c2f8f01 100644
--- a/aggregate_test.go
+++ b/aggregate_test.go
@@ -293,7 +293,7 @@ func TestDurationAggregation(t *testing.T) {
 			Value:      3,
 		},
 	}
-	err = table.Write(context.Background(), records...)
+	_, err = table.Write(context.Background(), records...)
 	require.NoError(t, err)
 
 	engine := query.NewEngine(memory.DefaultAllocator, db.TableProvider())
diff --git a/db_test.go b/db_test.go
index ad31d03..3dac5bb 100644
--- a/db_test.go
+++ b/db_test.go
@@ -1080,7 +1080,7 @@ func Test_DB_TableWrite_FlatSchema(t *testing.T) {
 	require.NoError(t, err)
 	defer table.Release()
 
-	err = table.Write(ctx, Flat{
+	_, err = table.Write(ctx, Flat{
 		ExampleType: "hello-world",
 		Timestamp:   7,
 		Value:       8,
@@ -1159,7 +1159,7 @@ func Test_DB_TableWrite_DynamicSchema(t *testing.T) {
 		},
 	}
 
-	err = table.Write(ctx, samples[0], samples[1], samples[2])
+	_, err = table.Write(ctx, samples[0], samples[1], samples[2])
 	require.NoError(t, err)
 
 	engine := query.NewEngine(
diff --git a/snapshot_test.go b/snapshot_test.go
index 3f8bf54..244e4b5 100644
--- a/snapshot_test.go
+++ b/snapshot_test.go
@@ -374,7 +374,7 @@ func TestSnapshotIsTakenOnUncompressedInserts(t *testing.T) {
 	defer table.Release()
 
 	for i := 0; i < numInserts; i++ {
-		err = table.Write(ctx, model{Bytes: "test"})
+		_, err = table.Write(ctx, model{Bytes: "test"})
 		require.NoError(t, err)
 	}
 	activeBlock := table.ActiveBlock()
@@ -391,7 +391,7 @@ func TestSnapshotIsTakenOnUncompressedInserts(t *testing.T) {
 	// These writes should now trigger a snapshot even though the active block
 	// size is much lower than the uncompressed insert size.
 	for i := 0; i < 2; i++ {
-		err = table.Write(ctx, model{Bytes: "test"})
+		_, err = table.Write(ctx, model{Bytes: "test"})
 		require.NoError(t, err)
 	}
 
diff --git a/table_test.go b/table_test.go
index 2969298..9984d39 100644
--- a/table_test.go
+++ b/table_test.go
@@ -1064,7 +1064,7 @@ func Test_Table_DynamicColumnMap(t *testing.T) {
 	require.NoError(t, err)
 	defer table.Release()
 
-	err = table.Write(context.Background(), ColMap{
+	_, err = table.Write(context.Background(), ColMap{
 		Name: "albert",
 		Attributes: map[string]string{
 			"age": "9999",
@@ -1094,7 +1094,7 @@ func Test_Table_DynamicColumnNotDefined(t *testing.T) {
 	require.NoError(t, err)
 	defer table.Release()
 
-	err = table.Write(context.Background(), ColMap{
+	_, err = table.Write(context.Background(), ColMap{
 		Name: "albert",
 	})
 	require.NoError(t, err)
@@ -1123,7 +1123,7 @@ func TestTableUniquePrimaryIndex(t *testing.T) {
 
 	const numRecords = 9
 	for i := 0; i < numRecords; i++ {
-		err = table.Write(context.Background(), Record{
+		_, err = table.Write(context.Background(), Record{
 			Name: "duplicate",
 		})
 		require.NoError(t, err)
@@ -1141,7 +1141,7 @@ func TestTableUniquePrimaryIndex(t *testing.T) {
 	require.Equal(t, numRecords, rowsRead)
 
 	// Trigger compaction with a new record.
-	err = table.Write(context.Background(), Record{
+	_, err = table.Write(context.Background(), Record{
 		Name: "duplicate",
 	})
 	require.NoError(t, err)
@@ -1174,7 +1174,7 @@ func TestTable_write_ptr_struct(t *testing.T) {
 	require.Nil(t, err)
 	defer table.Release()
 
-	err = table.Write(context.Background(), &dynparquet.Sample{
+	_, err = table.Write(context.Background(), &dynparquet.Sample{
 		ExampleType: "ptr",
 	})
 	require.Nil(t, err)