Merge pull request #13 from hugr-lab/013-batch-table-signature

feat: simplify batch table interfaces and add RemoveField

Author: VGSML

CLAUDE.md

@@ -12,6 +12,7 @@ Auto-generated from all feature plans. Last updated: 2025-11-29
 - N/A (storage-agnostic library) (001-batch-table-interfaces)
 - Go 1.25+ (matching existing project) + Standard library only (encoding/json for parsing, strings/fmt for encoding). No new external dependencies required. (012-filter-pushdown)
 - N/A (pure transformation library) (012-filter-pushdown)
+- Go 1.25+ + Apache Arrow Go v18 (`github.com/apache/arrow-go/v18`), gRPC (013-batch-table-signature)
 
 ## Project Structure
 
@@ -76,9 +77,9 @@ go work sync
 - No silent failures - errors must be handled explicitly
 
 ## Recent Changes
+- 013-batch-table-signature: Added Go 1.25+ + Apache Arrow Go v18 (`github.com/apache/arrow-go/v18`), gRPC
 - 012-filter-pushdown: Added Go 1.25+ (matching existing project) + Standard library only (encoding/json for parsing, strings/fmt for encoding). No new external dependencies required.
 - 001-batch-table-interfaces: Added Go 1.25+ + Apache Arrow Go v18, gRPC, msgpack-go
-- 006-returning-optimization: Added Go 1.25+ + Arrow-Go v18, gRPC, msgpack-go
 
 <!-- MANUAL ADDITIONS START -->
 <!-- MANUAL ADDITIONS END -->

catalog/dynamic.go

@@ -145,6 +145,12 @@ type RenameFieldOptions struct {
 	IgnoreNotFound bool
 }
 
+// RemoveFieldOptions configures field removal from struct columns.
+type RemoveFieldOptions struct {
+	// IgnoreNotFound suppresses error if table or column doesn't exist.
+	IgnoreNotFound bool
+}
+
 // CatalogVersion contains the version information for a catalog.
 type CatalogVersion struct {
 	// Version is the current version number of the catalog.
@@ -251,4 +257,9 @@ type DynamicTable interface {
 	// The columnPath is the path to the field (e.g., ["col", "nested", "field"]).
 	// Returns ErrNotFound if column path doesn't exist and IgnoreNotFound is false.
 	RenameField(ctx context.Context, columnPath []string, newName string, opts RenameFieldOptions) error
+
+	// RemoveField removes a field from a struct-typed column.
+	// The columnPath is the path to the field (e.g., ["col", "nested", "field"]).
+	// Returns ErrNotFound if column path doesn't exist and IfFieldExists is false.
+	RemoveField(ctx context.Context, columnPath []string, opts RemoveFieldOptions) error
 }

catalog/table.go

@@ -102,47 +102,49 @@ type DeletableTable interface {
 }
 
 // UpdatableBatchTable extends Table with batch-oriented UPDATE capability.
-// The Update method receives the complete input RecordReader including the rowid column.
-// Implementations extract rowid values from the rowid column in the RecordReader.
+// The Update method receives the complete input RecordBatch including the rowid column.
+// Implementations extract rowid values from the rowid column in the RecordBatch.
 // This interface is preferred over UpdatableTable when both are implemented.
 // Implementations MUST be goroutine-safe.
 type UpdatableBatchTable interface {
 	Table
 
-	// Update modifies existing rows using data from the RecordReader.
-	// The rows RecordReader contains both the rowid column (identifying rows to update)
+	// Update modifies existing rows using data from the RecordBatch.
+	// The rows RecordBatch contains both the rowid column (identifying rows to update)
 	// and the new column values. Implementations MUST extract rowid values from
 	// the rowid column (identified by name "rowid" or metadata key "is_rowid").
 	// Use FindRowIDColumn(rows.Schema()) to locate the rowid column.
-	// Row order in RecordReader determines update order.
+	// Implementations MUST return ErrNullRowID if any rowid value is null.
+	// Row order in RecordBatch determines update order.
 	// The opts parameter provides options including RETURNING clause information:
 	//   - opts.Returning: true if RETURNING clause was specified
 	//   - opts.ReturningColumns: column names to include in RETURNING results
 	// Returns DMLResult with affected row count and optional returning data.
 	// Context may contain transaction ID for coordinated operations.
 	// Caller MUST call rows.Release() after Update returns.
-	Update(ctx context.Context, rows array.RecordReader, opts *DMLOptions) (*DMLResult, error)
+	Update(ctx context.Context, rows arrow.RecordBatch, opts *DMLOptions) (*DMLResult, error)
 }
 
 // DeletableBatchTable extends Table with batch-oriented DELETE capability.
-// The Delete method receives a RecordReader containing the rowid column.
-// Implementations extract rowid values from the rowid column in the RecordReader.
+// The Delete method receives a RecordBatch containing the rowid column.
+// Implementations extract rowid values from the rowid column in the RecordBatch.
 // This interface is preferred over DeletableTable when both are implemented.
 // Implementations MUST be goroutine-safe.
 type DeletableBatchTable interface {
 	Table
 
-	// Delete removes rows identified by rowid values in the RecordReader.
-	// The rows RecordReader contains the rowid column (identified by name "rowid"
+	// Delete removes rows identified by rowid values in the RecordBatch.
+	// The rows RecordBatch contains the rowid column (identified by name "rowid"
 	// or metadata key "is_rowid") that identifies rows to delete.
 	// Use FindRowIDColumn(rows.Schema()) to locate the rowid column.
+	// Implementations MUST return ErrNullRowID if any rowid value is null.
 	// The opts parameter provides options including RETURNING clause information:
 	//   - opts.Returning: true if RETURNING clause was specified
 	//   - opts.ReturningColumns: column names to include in RETURNING results
 	// Returns DMLResult with affected row count and optional returning data.
 	// Context may contain transaction ID for coordinated operations.
 	// Caller MUST call rows.Release() after Delete returns.
-	Delete(ctx context.Context, rows array.RecordReader, opts *DMLOptions) (*DMLResult, error)
+	Delete(ctx context.Context, rows arrow.RecordBatch, opts *DMLOptions) (*DMLResult, error)
 }
 
 // ColumnStats contains statistics for a single table column.

catalog/types.go

@@ -2,11 +2,17 @@ package catalog
 
 import (
 	"context"
+	"errors"
 
 	"github.com/apache/arrow-go/v18/arrow"
 	"github.com/apache/arrow-go/v18/arrow/array"
 )
 
+// ErrNullRowID is returned when a null rowid value is encountered in UPDATE or DELETE operations.
+// Implementations of UpdatableBatchTable and DeletableBatchTable MUST return this error
+// when they encounter null values in the rowid column of the input Record.
+var ErrNullRowID = errors.New("null rowid value not allowed")
+
 // ScanOptions provides options for table scans.
 type ScanOptions struct {
 	// Columns to return. If nil/empty, all columns are projected.

docs/api-guide.md

@@ -231,46 +231,57 @@ type DeletableTable interface {
 
 ### catalog.UpdatableBatchTable
 
-Alternative UPDATE interface where the rowid column is embedded in the RecordReader.
+Alternative UPDATE interface where the rowid column is embedded in the RecordBatch.
 This interface is preferred over `UpdatableTable` when both are implemented.
 
 ```go
 type UpdatableBatchTable interface {
     Table
 
-    // Update modifies existing rows using data from the RecordReader.
-    // The rows RecordReader contains both the rowid column (identifying rows to update)
+    // Update modifies existing rows using data from the RecordBatch.
+    // The rows RecordBatch contains both the rowid column (identifying rows to update)
     // and the new column values. Implementations MUST extract rowid values from
     // the rowid column (identified by name "rowid" or metadata key "is_rowid").
     // Use FindRowIDColumn(rows.Schema()) to locate the rowid column.
-    // Row order in RecordReader determines update order.
+    // Implementations MUST return ErrNullRowID if any rowid value is null.
+    // Row order in RecordBatch determines update order.
     // opts contains RETURNING clause information.
     // Returns DMLResult with affected row count and optional returning data.
     // Caller MUST call rows.Release() after Update returns.
-    Update(ctx context.Context, rows array.RecordReader, opts *DMLOptions) (*DMLResult, error)
+    Update(ctx context.Context, rows arrow.RecordBatch, opts *DMLOptions) (*DMLResult, error)
 }
 ```
 
 ### catalog.DeletableBatchTable
 
-Alternative DELETE interface where the rowid column is embedded in the RecordReader.
+Alternative DELETE interface where the rowid column is embedded in the RecordBatch.
 This interface is preferred over `DeletableTable` when both are implemented.
 
 ```go
 type DeletableBatchTable interface {
     Table
 
-    // Delete removes rows identified by rowid values in the RecordReader.
-    // The rows RecordReader contains the rowid column (identified by name "rowid"
+    // Delete removes rows identified by rowid values in the RecordBatch.
+    // The rows RecordBatch contains the rowid column (identified by name "rowid"
     // or metadata key "is_rowid") that identifies rows to delete.
     // Use FindRowIDColumn(rows.Schema()) to locate the rowid column.
+    // Implementations MUST return ErrNullRowID if any rowid value is null.
     // opts contains RETURNING clause information.
     // Returns DMLResult with affected row count and optional returning data.
     // Caller MUST call rows.Release() after Delete returns.
-    Delete(ctx context.Context, rows array.RecordReader, opts *DMLOptions) (*DMLResult, error)
+    Delete(ctx context.Context, rows arrow.RecordBatch, opts *DMLOptions) (*DMLResult, error)
 }
 ```
 
+### catalog.ErrNullRowID
+
+Sentinel error for null rowid values:
+
+```go
+// ErrNullRowID is returned when a null rowid value is encountered in UPDATE or DELETE operations.
+var ErrNullRowID = errors.New("null rowid value not allowed")
+```
+
 ### catalog.FindRowIDColumn
 
 Helper function to locate the rowid column in a schema:
@@ -288,21 +299,26 @@ func FindRowIDColumn(schema *arrow.Schema) int
 **Example usage:**
 
 ```go
-func (t *MyTable) Update(ctx context.Context, rows array.RecordReader, opts *catalog.DMLOptions) (*catalog.DMLResult, error) {
+func (t *MyTable) Update(ctx context.Context, rows arrow.RecordBatch, opts *catalog.DMLOptions) (*catalog.DMLResult, error) {
     rowidIdx := catalog.FindRowIDColumn(rows.Schema())
     if rowidIdx == -1 {
         return nil, errors.New("rowid column required")
     }
 
-    var affected int64
-    for rows.Next() {
-        batch := rows.RecordBatch()
-        rowidCol := batch.Column(rowidIdx).(*array.Int64)
-        // Process updates using rowidCol values...
-        affected += batch.NumRows()
+    // Check for null rowids
+    rowidCol := rows.Column(rowidIdx)
+    if rowidCol.NullN() > 0 {
+        return nil, catalog.ErrNullRowID
     }
 
-    return &catalog.DMLResult{AffectedRows: affected}, nil
+    // Direct access to RecordBatch - no iterator needed
+    rowidArr := rowidCol.(*array.Int64)
+    for i := 0; i < int(rows.NumRows()); i++ {
+        rowID := rowidArr.Value(i)
+        // Process update for rowID...
+    }
+
+    return &catalog.DMLResult{AffectedRows: rows.NumRows()}, nil
 }
 ```
 
@@ -500,6 +516,9 @@ type DynamicTable interface {
 
     // RenameField renames a field in a struct-typed column.
     RenameField(ctx context.Context, columnPath []string, newName string, opts RenameFieldOptions) error
+
+    // RemoveField removes a field from a struct-typed column.
+    RemoveField(ctx context.Context, columnPath []string, opts RemoveFieldOptions) error
 }
 
 type AddColumnOptions struct {
@@ -541,6 +560,10 @@ type AddFieldOptions struct {
 type RenameFieldOptions struct {
     IgnoreNotFound bool
 }
+
+type RemoveFieldOptions struct {
+    IgnoreNotFound bool // Don't error if table/column doesn't exist
+}
 ```
 
 ## Statistics Interface

examples/ddl/main.go

@@ -628,3 +628,16 @@ func (t *DDLTable) RenameField(_ context.Context, columnPath []string, newName s
 	fmt.Printf("[DDLTable:%s] RenameField called: %v -> %s (simplified)\n", t.name, columnPath, newName)
 	return nil
 }
+
+// RemoveField implements catalog.DynamicTable.
+func (t *DDLTable) RemoveField(_ context.Context, columnPath []string, opts catalog.RemoveFieldOptions) error {
+	// Simplified implementation - real implementation would handle nested struct fields
+	if len(columnPath) == 0 {
+		if opts.IgnoreNotFound {
+			return nil
+		}
+		return catalog.ErrNotFound
+	}
+	fmt.Printf("[DDLTable:%s] RemoveField called: %v (simplified)\n", t.name, columnPath)
+	return nil
+}

examples/dml/main.go

@@ -216,9 +216,10 @@ func (m *InMemoryTransactionManager) GetTransactionStatus(_ context.Context, txI
 // and catalog.DeletableTable interfaces.
 //
 // Alternative: You can implement catalog.UpdatableBatchTable and catalog.DeletableBatchTable
-// instead (or in addition). The batch interfaces receive rowid embedded in the RecordReader
-// rather than as a separate []int64 slice. Use catalog.FindRowIDColumn(rows.Schema())
-// to locate the rowid column. Batch interfaces are preferred when both are implemented.
+// instead (or in addition). The batch interfaces receive a single arrow.RecordBatch containing
+// all data including the rowid column, rather than a separate []int64 slice for rowids.
+// Use catalog.FindRowIDColumn(rows.Schema()) to locate the rowid column.
+// Batch interfaces are preferred when both are implemented.
 type UsersTable struct {
 	schema    *arrow.Schema
 	alloc     memory.Allocator

flight/doaction.go

@@ -69,6 +69,8 @@ func (s *Server) DoAction(action *flight.Action, stream flight.FlightService_DoA
 		return s.handleAddFieldAction(ctx, action, stream)
 	case "rename_field":
 		return s.handleRenameFieldAction(ctx, action, stream)
+	case "remove_field":
+		return s.handleRemoveFieldAction(ctx, action, stream)
 
 	// Catalog version action
 	case "catalog_version":