review comments

substrait-io · Jan 16, 2025 · 574a317 · 574a317
1 parent 602f464
commit 574a317
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 0 deletions.
diff --git a/plan/plan.go b/plan/plan.go
@@ -397,7 +397,11 @@ func RelFromProto(rel *proto.Rel, reg expr.ExtensionRegistry) (Rel, error) {
 				out = &IcebergTableReadRel{
 					tableType: tableType,
 				}
+			} else {
+				return nil, fmt.Errorf("%w: only IcebergTableType Direct is supported", substraitgo.ErrInvalidRel)
 			}
+		default:
+			return nil, fmt.Errorf("%w: unknown ReadRel type", substraitgo.ErrInvalidRel)
 		}
 
 		if err := out.fromProtoReadRel(rel.Read, reg); err != nil {

diff --git a/plan/relations_test.go b/plan/relations_test.go
@@ -43,6 +43,12 @@ func TestRelations_Copy(t *testing.T) {
 	sortRel := &SortRel{input: createVirtualTableReadRel(1), sorts: []expr.SortField{{Expr: createPrimitiveFloat(1.0), Kind: types.SortAscNullsFirst}}}
 	virtualTableReadRel := &VirtualTableReadRel{values: []expr.VirtualTableExpressionValue{[]expr.Expression{&expr.PrimitiveLiteral[int64]{Value: 1}}}}
 	namedTableWriteRel := &NamedTableWriteRel{input: namedTableReadRel}
+	icebergTableReadRel := &IcebergTableReadRel{
+		baseReadRel: baseReadRel{filter: expr.NewPrimitiveLiteral(true, false)},
+		tableType: &Direct{
+			MetadataUri: "s3://bucket/path/to/metadata.json",
+		},
+	}
 
 	type relationTestCase struct {
 		name            string
@@ -324,6 +330,27 @@ func TestRelations_Copy(t *testing.T) {
 			rewriteFunc: func(e expr.Expression) (expr.Expression, error) { return createPrimitiveFloat(9.0), nil },
 			expectedRel: &NamedTableWriteRel{input: createVirtualTableReadRel(6)},
 		},
+		{
+			name:            "IcebergTableReadRel Copy with new inputs",
+			relation:        icebergTableReadRel,
+			newInputs:       []Rel{},
+			expectedSameRel: true,
+		},
+		{
+			name:        "IcebergTableReadRel Copy with new inputs and rewriteFunc",
+			relation:    icebergTableReadRel,
+			newInputs:   []Rel{},
+			rewriteFunc: func(e expr.Expression) (expr.Expression, error) { return createPrimitiveFloat(9.0), nil },
+			expectedRel: &IcebergTableReadRel{
+				baseReadRel: baseReadRel{
+					filter:           createPrimitiveFloat(9.0),
+					bestEffortFilter: createPrimitiveFloat(9.0),
+				},
+				tableType: &Direct{
+					MetadataUri: "s3://bucket/path/to/metadata.json",
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {