Skip to content

Commit

Permalink
Revert "[COR-2297/] Fix nested offloaded type validation (#552) (#5996)"
Browse files Browse the repository at this point in the history 
This reverts commit f20b8aa.

Signed-off-by: Eduardo Apolinario <[email protected]>
  • Loading branch information
@eapolinario
eapolinario committed Nov 21, 2024
1 parent e13babb commit d8849d6
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 116 deletions.
25 changes: 6 additions & 19 deletions flytepropeller/pkg/compiler/validators/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,18 +202,13 @@ func buildMultipleTypeUnion(innerType []*core.LiteralType) *core.LiteralType {
return unionLiteralType
}

func literalTypeForLiterals(literals []*core.Literal) (*core.LiteralType, bool) {
func literalTypeForLiterals(literals []*core.Literal) *core.LiteralType {
innerType := make([]*core.LiteralType, 0, 1)
innerTypeSet := sets.NewString()
var noneType *core.LiteralType
isOffloadedType := false
for _, x := range literals {
otherType := LiteralTypeForLiteral(x)
otherTypeKey := otherType.String()
if _, ok := x.GetValue().(*core.Literal_OffloadedMetadata); ok {
isOffloadedType = true
return otherType, isOffloadedType
}
if _, ok := x.GetValue().(*core.Literal_Collection); ok {
if x.GetCollection().GetLiterals() == nil {
noneType = otherType
Expand All @@ -235,9 +230,9 @@ func literalTypeForLiterals(literals []*core.Literal) (*core.LiteralType, bool)
if len(innerType) == 0 {
return &core.LiteralType{
Type: &core.LiteralType_Simple{Simple: core.SimpleType_NONE},
}, isOffloadedType
}
} else if len(innerType) == 1 {
return innerType[0], isOffloadedType
return innerType[0]
}

// sort inner types to ensure consistent union types are generated
Expand All @@ -252,7 +247,7 @@ func literalTypeForLiterals(literals []*core.Literal) (*core.LiteralType, bool)

return 0
})
return buildMultipleTypeUnion(innerType), isOffloadedType
return buildMultipleTypeUnion(innerType)
}

// ValidateLiteralType check if the literal type is valid, return error if the literal is invalid.
Expand All @@ -279,23 +274,15 @@ func LiteralTypeForLiteral(l *core.Literal) *core.LiteralType {
case *core.Literal_Scalar:
return literalTypeForScalar(l.GetScalar())
case *core.Literal_Collection:
collectionType, isOffloaded := literalTypeForLiterals(l.GetCollection().GetLiterals())
if isOffloaded {
return collectionType
}
return &core.LiteralType{
Type: &core.LiteralType_CollectionType{
CollectionType: collectionType,
CollectionType: literalTypeForLiterals(l.GetCollection().Literals),
},
}
case *core.Literal_Map:
mapValueType, isOffloaded := literalTypeForLiterals(maps.Values(l.GetMap().GetLiterals()))
if isOffloaded {
return mapValueType
}
return &core.LiteralType{
Type: &core.LiteralType_MapValueType{
MapValueType: mapValueType,
MapValueType: literalTypeForLiterals(maps.Values(l.GetMap().Literals)),
},
}
case *core.Literal_OffloadedMetadata:
Expand Down
107 changes: 10 additions & 97 deletions flytepropeller/pkg/compiler/validators/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ import (

func TestLiteralTypeForLiterals(t *testing.T) {
t.Run("empty", func(t *testing.T) {
lt, isOffloaded := literalTypeForLiterals(nil)
lt := literalTypeForLiterals(nil)
assert.Equal(t, core.SimpleType_NONE.String(), lt.GetSimple().String())
assert.False(t, isOffloaded)
})

t.Run("binary idl with raw binary data and no tag", func(t *testing.T) {
Expand Down Expand Up @@ -95,43 +94,40 @@ func TestLiteralTypeForLiterals(t *testing.T) {
})

t.Run("homogeneous", func(t *testing.T) {
lt, isOffloaded := literalTypeForLiterals([]*core.Literal{
lt := literalTypeForLiterals([]*core.Literal{
coreutils.MustMakeLiteral(5),
coreutils.MustMakeLiteral(0),
coreutils.MustMakeLiteral(5),
})

assert.Equal(t, core.SimpleType_INTEGER.String(), lt.GetSimple().String())
assert.False(t, isOffloaded)
})

t.Run("non-homogenous", func(t *testing.T) {
lt, isOffloaded := literalTypeForLiterals([]*core.Literal{
lt := literalTypeForLiterals([]*core.Literal{
coreutils.MustMakeLiteral("hello"),
coreutils.MustMakeLiteral(5),
coreutils.MustMakeLiteral("world"),
coreutils.MustMakeLiteral(0),
coreutils.MustMakeLiteral(2),
})

assert.Len(t, lt.GetUnionType().GetVariants(), 2)
assert.Equal(t, core.SimpleType_INTEGER.String(), lt.GetUnionType().GetVariants()[0].GetSimple().String())
assert.Equal(t, core.SimpleType_STRING.String(), lt.GetUnionType().GetVariants()[1].GetSimple().String())
assert.False(t, isOffloaded)
assert.Len(t, lt.GetUnionType().Variants, 2)
assert.Equal(t, core.SimpleType_INTEGER.String(), lt.GetUnionType().Variants[0].GetSimple().String())
assert.Equal(t, core.SimpleType_STRING.String(), lt.GetUnionType().Variants[1].GetSimple().String())
})

t.Run("non-homogenous ensure ordering", func(t *testing.T) {
lt, isOffloaded := literalTypeForLiterals([]*core.Literal{
lt := literalTypeForLiterals([]*core.Literal{
coreutils.MustMakeLiteral(5),
coreutils.MustMakeLiteral("world"),
coreutils.MustMakeLiteral(0),
coreutils.MustMakeLiteral(2),
})

assert.Len(t, lt.GetUnionType().GetVariants(), 2)
assert.Equal(t, core.SimpleType_INTEGER.String(), lt.GetUnionType().GetVariants()[0].GetSimple().String())
assert.Equal(t, core.SimpleType_STRING.String(), lt.GetUnionType().GetVariants()[1].GetSimple().String())
assert.False(t, isOffloaded)
assert.Len(t, lt.GetUnionType().Variants, 2)
assert.Equal(t, core.SimpleType_INTEGER.String(), lt.GetUnionType().Variants[0].GetSimple().String())
assert.Equal(t, core.SimpleType_STRING.String(), lt.GetUnionType().Variants[1].GetSimple().String())
})

t.Run("list with mixed types", func(t *testing.T) {
Expand Down Expand Up @@ -458,89 +454,6 @@ func TestLiteralTypeForLiterals(t *testing.T) {
assert.True(t, proto.Equal(expectedLt, lt))
})

t.Run("nested Lists of offloaded List of string types", func(t *testing.T) {
inferredType := &core.LiteralType{
Type: &core.LiteralType_CollectionType{
CollectionType: &core.LiteralType{
Type: &core.LiteralType_Simple{
Simple: core.SimpleType_STRING,
},
},
},
}
literals := &core.Literal{
Value: &core.Literal_Collection{
Collection: &core.LiteralCollection{
Literals: []*core.Literal{
{
Value: &core.Literal_OffloadedMetadata{
OffloadedMetadata: &core.LiteralOffloadedMetadata{
Uri: "dummy/uri-1",
SizeBytes: 1000,
InferredType: inferredType,
},
},
},
{
Value: &core.Literal_OffloadedMetadata{
OffloadedMetadata: &core.LiteralOffloadedMetadata{
Uri: "dummy/uri-2",
SizeBytes: 1000,
InferredType: inferredType,
},
},
},
},
},
},
}
expectedLt := inferredType
lt := LiteralTypeForLiteral(literals)
assert.True(t, proto.Equal(expectedLt, lt))
})
t.Run("nested map of offloaded map of string types", func(t *testing.T) {
inferredType := &core.LiteralType{
Type: &core.LiteralType_MapValueType{
MapValueType: &core.LiteralType{
Type: &core.LiteralType_Simple{
Simple: core.SimpleType_STRING,
},
},
},
}
literals := &core.Literal{
Value: &core.Literal_Map{
Map: &core.LiteralMap{
Literals: map[string]*core.Literal{

"key1": {
Value: &core.Literal_OffloadedMetadata{
OffloadedMetadata: &core.LiteralOffloadedMetadata{
Uri: "dummy/uri-1",
SizeBytes: 1000,
InferredType: inferredType,
},
},
},
"key2": {
Value: &core.Literal_OffloadedMetadata{
OffloadedMetadata: &core.LiteralOffloadedMetadata{
Uri: "dummy/uri-2",
SizeBytes: 1000,
InferredType: inferredType,
},
},
},
},
},
},
}

expectedLt := inferredType
lt := LiteralTypeForLiteral(literals)
assert.True(t, proto.Equal(expectedLt, lt))
})

}

func TestJoinVariableMapsUniqueKeys(t *testing.T) {
Expand Down

0 comments on commit d8849d6

Please sign in to comment.