Copilot commented on code in PR #2301:
URL: https://github.com/apache/iceberg-go/pull/2301#discussion_r4361059814


##########
catalog/sql/sql_test.go:
##########
@@ -1496,6 +1497,245 @@ func (s *SqliteCatalogTestSuite) 
TestDropTableNotExist() {
        }
 }
 
+// stageStringRow stages one row for the string field "foo" on tx.
+func (s *SqliteCatalogTestSuite) stageStringRow(ctx context.Context, tx 
*table.Transaction, value string) {
+       arrowSchema := arrow.NewSchema([]arrow.Field{{Name: "foo", Type: 
arrow.BinaryTypes.String}}, nil)
+       bldr := array.NewStringBuilder(memory.DefaultAllocator)
+       defer bldr.Release()
+       bldr.Append(value)
+       arr := bldr.NewArray()
+       defer arr.Release()
+       rec := array.NewRecordBatch(arrowSchema, []arrow.Array{arr}, 1)
+       defer rec.Release()
+       arrTable := array.NewTableFromRecords(arrowSchema, 
[]arrow.RecordBatch{rec})
+       defer arrTable.Release()
+
+       s.Require().NoError(tx.AppendTable(ctx, arrTable, 1024, nil))
+}
+
+// appendStringRow commits one row to branch, creating the branch from
+// main's head if it does not exist.
+func (s *SqliteCatalogTestSuite) appendStringRow(ctx context.Context, tbl 
*table.Table, branch, value string) *table.Table {
+       tx := tbl.NewTransactionOnBranch(branch)
+       s.stageStringRow(ctx, tx, value)
+       out, err := tx.Commit(ctx)
+       s.Require().NoError(err)
+
+       return out
+}
+
+// waitPastSnapshots blocks until the wall clock in milliseconds is past
+// every snapshot timestamp of meta, so an expiry with WithOlderThan(0)
+// treats them all as old enough.
+func (s *SqliteCatalogTestSuite) waitPastSnapshots(meta table.Metadata) {
+       var newest int64
+       for _, snap := range meta.Snapshots() {
+               newest = max(newest, snap.TimestampMs)

Review Comment:
   The `max` builtin is only available in Go 1.21+. If this repo/module targets 
an older Go version, this will not compile. To keep the test compatible, 
replace `max(...)` with an explicit comparison assignment.



##########
catalog/sql/sql_test.go:
##########
@@ -1496,6 +1497,245 @@ func (s *SqliteCatalogTestSuite) 
TestDropTableNotExist() {
        }
 }
 
+// stageStringRow stages one row for the string field "foo" on tx.
+func (s *SqliteCatalogTestSuite) stageStringRow(ctx context.Context, tx 
*table.Transaction, value string) {
+       arrowSchema := arrow.NewSchema([]arrow.Field{{Name: "foo", Type: 
arrow.BinaryTypes.String}}, nil)
+       bldr := array.NewStringBuilder(memory.DefaultAllocator)
+       defer bldr.Release()
+       bldr.Append(value)
+       arr := bldr.NewArray()
+       defer arr.Release()
+       rec := array.NewRecordBatch(arrowSchema, []arrow.Array{arr}, 1)
+       defer rec.Release()
+       arrTable := array.NewTableFromRecords(arrowSchema, 
[]arrow.RecordBatch{rec})

Review Comment:
   `array.NewRecordBatch` / `arrow.RecordBatch` are not part of the Apache 
Arrow Go API (records are typically `arrow.Record` created via 
`array.NewRecord`, and tables are constructed from `[]arrow.Record`). As 
written, this is likely to fail to compile; update this helper to build an 
`arrow.Record` and pass `[]arrow.Record` into `array.NewTableFromRecords`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to