pdelewski commented on code in PR #408:
URL: https://github.com/apache/iceberg-go/pull/408#discussion_r2066816991


##########
table/table_test.go:
##########
@@ -1135,3 +1136,73 @@ func TestTableWriting(t *testing.T) {
        suite.Run(t, &TableWritingTestSuite{formatVersion: 1})
        suite.Run(t, &TableWritingTestSuite{formatVersion: 2})
 }
+
+func TestNullableStructRequiredField(t *testing.T) {
+       loc := t.TempDir()
+
+       cat, err := catalog.Load(context.Background(), "default", 
iceberg.Properties{
+               "uri":          ":memory:",
+               "type":         "sql",
+               sql.DriverKey:  sqliteshim.ShimName,
+               sql.DialectKey: string(sql.SQLite),
+               "warehouse":    "file://" + loc,
+       })
+       require.NoError(t, err)
+
+       arrowSchema := arrow.NewSchema([]arrow.Field{
+               {
+                       Name: "analytic", Type: arrow.StructOf(
+                               arrow.Field{Name: "category", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                               arrow.Field{Name: "desc", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                               arrow.Field{Name: "name", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                               arrow.Field{Name: "related_analytics", Type: 
arrow.ListOf(
+                                       arrow.StructOf(
+                                               arrow.Field{Name: "category", 
Type: arrow.BinaryTypes.String, Nullable: true},
+                                               arrow.Field{Name: "desc", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                                               arrow.Field{Name: "name", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                                               arrow.Field{Name: "type", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                                               arrow.Field{Name: "type_id", 
Type: arrow.PrimitiveTypes.Int32, Nullable: false},
+                                               arrow.Field{Name: "uid", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                                               arrow.Field{Name: "version", 
Type: arrow.BinaryTypes.String, Nullable: true},
+                                       ),
+                               ), Nullable: true},
+                               arrow.Field{Name: "type", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                               arrow.Field{Name: "type_id", Type: 
arrow.PrimitiveTypes.Int32, Nullable: false},
+                               arrow.Field{Name: "uid", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                               arrow.Field{Name: "version", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                       ), Nullable: true,
+               },
+               {Name: "uid", Type: arrow.BinaryTypes.String, Nullable: true},
+       }, nil)
+
+       sc, err := table.ArrowSchemaToIcebergWithoutIDs(arrowSchema, false)
+       if err != nil {
+               panic(err)

Review Comment:
   is any reason to use `panic` here instead of `require.NoError(t, err)`



##########
table/table_test.go:
##########
@@ -1135,3 +1136,73 @@ func TestTableWriting(t *testing.T) {
        suite.Run(t, &TableWritingTestSuite{formatVersion: 1})
        suite.Run(t, &TableWritingTestSuite{formatVersion: 2})
 }
+
+func TestNullableStructRequiredField(t *testing.T) {
+       loc := t.TempDir()
+
+       cat, err := catalog.Load(context.Background(), "default", 
iceberg.Properties{
+               "uri":          ":memory:",
+               "type":         "sql",
+               sql.DriverKey:  sqliteshim.ShimName,
+               sql.DialectKey: string(sql.SQLite),
+               "warehouse":    "file://" + loc,
+       })
+       require.NoError(t, err)
+
+       arrowSchema := arrow.NewSchema([]arrow.Field{
+               {
+                       Name: "analytic", Type: arrow.StructOf(
+                               arrow.Field{Name: "category", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                               arrow.Field{Name: "desc", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                               arrow.Field{Name: "name", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                               arrow.Field{Name: "related_analytics", Type: 
arrow.ListOf(
+                                       arrow.StructOf(
+                                               arrow.Field{Name: "category", 
Type: arrow.BinaryTypes.String, Nullable: true},
+                                               arrow.Field{Name: "desc", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                                               arrow.Field{Name: "name", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                                               arrow.Field{Name: "type", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                                               arrow.Field{Name: "type_id", 
Type: arrow.PrimitiveTypes.Int32, Nullable: false},
+                                               arrow.Field{Name: "uid", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                                               arrow.Field{Name: "version", 
Type: arrow.BinaryTypes.String, Nullable: true},
+                                       ),
+                               ), Nullable: true},
+                               arrow.Field{Name: "type", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                               arrow.Field{Name: "type_id", Type: 
arrow.PrimitiveTypes.Int32, Nullable: false},
+                               arrow.Field{Name: "uid", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                               arrow.Field{Name: "version", Type: 
arrow.BinaryTypes.String, Nullable: true},
+                       ), Nullable: true,
+               },
+               {Name: "uid", Type: arrow.BinaryTypes.String, Nullable: true},
+       }, nil)
+
+       sc, err := table.ArrowSchemaToIcebergWithoutIDs(arrowSchema, false)
+       if err != nil {
+               panic(err)

Review Comment:
   is any reason to use `panic` here instead of `require.NoError(t, err)`?



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to