This is an automated email from the ASF dual-hosted git repository.
laskoviymishka pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new b3888c308 fix(substrait): preserve nested reference paths (#1588)
b3888c308 is described below
commit b3888c308e41717c145bada42c014fe3e6705d7d
Author: Minh Vu <[email protected]>
AuthorDate: Thu Jul 30 10:01:55 2026 +0200
fix(substrait): preserve nested reference paths (#1588)
## What changed
Build Substrait field references from the positional path retained by
the bound Iceberg reference. Add coverage for a colliding top-level leaf
name, a missing top-level leaf name, and a deeply nested field.
## Why
The converter rebound only the leaf field name. Nested references could
therefore target a same-named top-level column or fail conversion when
no top-level field shared that name.
## Testing
- `go test ./table/substrait`
Signed-off-by: Minh Vu <[email protected]>
---
table/substrait/substrait.go | 23 +++++------------
table/substrait/substrait_test.go | 54 +++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+), 16 deletions(-)
diff --git a/table/substrait/substrait.go b/table/substrait/substrait.go
index be6e026a9..26b03ed44 100644
--- a/table/substrait/substrait.go
+++ b/table/substrait/substrait.go
@@ -51,9 +51,10 @@ func NewExtensionSet() exprs.ExtensionIDSet {
return
exprs.NewExtensionSetDefault(expr.NewEmptyExtensionRegistry(collection))
}
-// ConvertExpr binds the provided expression to the given schema and converts
it to a
-// substrait expression so that it can be utilized for computation.
-func ConvertExpr(schema *iceberg.Schema, e iceberg.BooleanExpression,
caseSensitive bool) (*expr.ExtensionRegistry, expr.Expression, error) {
+// ConvertExpr converts a bound expression to a Substrait expression so that
it can
+// be utilized for computation. Case sensitivity is applied when binding; the
third
+// argument is retained for API compatibility.
+func ConvertExpr(schema *iceberg.Schema, e iceberg.BooleanExpression, _ bool)
(*expr.ExtensionRegistry, expr.Expression, error) {
base, err := ConvertSchema(schema)
if err != nil {
return nil, nil, err
@@ -62,10 +63,7 @@ func ConvertExpr(schema *iceberg.Schema, e
iceberg.BooleanExpression, caseSensit
reg := expr.NewEmptyExtensionRegistry(collection)
bldr := expr.ExprBuilder{Reg: reg, BaseSchema:
types.NewRecordTypeFromStruct(base.Struct)}
- b, err := iceberg.VisitExpr(e, &toSubstraitExpr{
- bldr: bldr, schema: schema,
- caseSensitive: caseSensitive,
- })
+ b, err := iceberg.VisitExpr(e, &toSubstraitExpr{bldr: bldr})
if err != nil {
return nil, nil, err
}
@@ -213,9 +211,7 @@ var (
)
type toSubstraitExpr struct {
- schema *iceberg.Schema
- bldr expr.ExprBuilder
- caseSensitive bool
+ bldr expr.ExprBuilder
}
func (t *toSubstraitExpr) VisitTrue() expr.Builder {
@@ -348,12 +344,7 @@ func toSubstraitLiteralSet(typ iceberg.Type, lits
[]iceberg.Literal) expr.ListLi
}
func (t *toSubstraitExpr) getRef(ref iceberg.BoundReference) expr.Reference {
- updatedRef, err := iceberg.Reference(ref.Field().Name).Bind(t.schema,
t.caseSensitive)
- if err != nil {
- panic(err)
- }
-
- path := updatedRef.Ref().PosPath()
+ path := ref.PosPath()
out := expr.NewStructFieldRef(int32(path[0]))
if len(path) == 1 {
return out
diff --git a/table/substrait/substrait_test.go
b/table/substrait/substrait_test.go
index e42ceeb1d..5c4f4ade5 100644
--- a/table/substrait/substrait_test.go
+++ b/table/substrait/substrait_test.go
@@ -79,6 +79,60 @@ func TestRefTypes(t *testing.T) {
}
}
+func TestNestedReferencesPreserveBoundPath(t *testing.T) {
+ sc := iceberg.NewSchema(1,
+ iceberg.NestedField{ID: 1, Name: "id", Type:
iceberg.PrimitiveTypes.Int64},
+ iceberg.NestedField{ID: 2, Name: "customer", Type:
&iceberg.StructType{FieldList: []iceberg.NestedField{
+ {ID: 3, Name: "id", Type: iceberg.PrimitiveTypes.Int64},
+ {ID: 4, Name: "address", Type:
&iceberg.StructType{FieldList: []iceberg.NestedField{
+ {ID: 5, Name: "zip", Type:
iceberg.PrimitiveTypes.String},
+ }}},
+ }}},
+ )
+
+ tests := []struct {
+ name string
+ predicate iceberg.UnboundPredicate
+ expected string
+ }{
+ {
+ name: "customer.id",
+ predicate:
iceberg.EqualTo(iceberg.Reference("customer.id"), int64(123)),
+ expected: ".field(1).field(0)",
+ },
+ {
+ name: "customer.address.zip",
+ predicate:
iceberg.EqualTo(iceberg.Reference("customer.address.zip"), "12345"),
+ expected: ".field(1).field(1).field(0)",
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ bound, err := tt.predicate.Bind(sc, true)
+ require.NoError(t, err)
+
+ _, converted, err := substrait.ConvertExpr(sc, bound,
true)
+ require.NoError(t, err)
+ assert.Contains(t, converted.String(), tt.expected)
+ })
+ }
+}
+
+func TestNestedReferenceWithoutTopLevelLeafName(t *testing.T) {
+ sc := iceberg.NewSchema(1,
+ iceberg.NestedField{ID: 1, Name: "customer", Type:
&iceberg.StructType{FieldList: []iceberg.NestedField{
+ {ID: 2, Name: "id", Type: iceberg.PrimitiveTypes.Int64},
+ }}},
+ )
+ bound, err := iceberg.EqualTo(iceberg.Reference("customer.id"),
int64(123)).Bind(sc, true)
+ require.NoError(t, err)
+
+ _, converted, err := substrait.ConvertExpr(sc, bound, true)
+ require.NoError(t, err)
+ assert.Contains(t, converted.String(), ".field(0).field(0)")
+}
+
var (
tableSchemaSimple = iceberg.NewSchemaWithIdentifiers(1,
[]int{2},
