Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
Make IntervalDayType as as separate type
  • Loading branch information
anshuldata committed Oct 21, 2024
1 parent 132dc66 commit 65f92ce
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 17 deletions.
6 changes: 4 additions & 2 deletions expr/literals.go
Original file line number Diff line number Diff line change
Expand Up @@ -812,21 +812,23 @@ func LiteralFromProto(l *proto.Expression_Literal) Literal {
Nullability: nullability,
}}
case *proto.Expression_Literal_IntervalDayToSecond_:
precision := types.PrecisionMicroSeconds
var precision types.TimePrecision
switch lit.IntervalDayToSecond.PrecisionMode.(type) {
case *proto.Expression_Literal_IntervalDayToSecond_Precision:
var err error
precision, err = types.ProtoToTimePrecision(lit.IntervalDayToSecond.GetPrecision())
if err != nil {
return nil
}
case *proto.Expression_Literal_IntervalDayToSecond_Microseconds:
precision = types.PrecisionMicroSeconds
default:
return nil
}
return &ProtoLiteral{
Value: lit.IntervalDayToSecond,
Type: &types.IntervalDayType{
Length: precision.ToProtoVal(),
Precision: precision,
Nullability: nullability,
TypeVariationRef: l.TypeVariationReference,
},
Expand Down
2 changes: 1 addition & 1 deletion expr/proto_literals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestLiteralFromProtoLiteral(t *testing.T) {
},
{"IntervalDayType",
&proto.Expression_Literal{LiteralType: &proto.Expression_Literal_IntervalDayToSecond_{IntervalDayToSecond: intDayToSecVal}, Nullable: true},
&ProtoLiteral{Value: intDayToSecVal, Type: &types.IntervalDayType{Length: types.PrecisionEMinus5Seconds.ToProtoVal(), Nullability: types.NullabilityNullable}},
&ProtoLiteral{Value: intDayToSecVal, Type: &types.IntervalDayType{Precision: types.PrecisionEMinus5Seconds, Nullability: types.NullabilityNullable}},
},
{"IntervalYearToMonthType",
&proto.Expression_Literal{LiteralType: &proto.Expression_Literal_IntervalYearToMonth_{IntervalYearToMonth: &proto.Expression_Literal_IntervalYearToMonth{Years: 1234, Months: 5}}, Nullable: true},
Expand Down
77 changes: 77 additions & 0 deletions types/interval_day_type.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package types

import (
"fmt"

"github.com/substrait-io/substrait-go/proto"
)

// IntervalDayType this is used to represent a type of interval day.
type IntervalDayType struct {
Precision TimePrecision
TypeVariationRef uint32
Nullability Nullability
}

func (m *IntervalDayType) WithTypeVariationRef(typeVariationRef uint32) *IntervalDayType {
m.TypeVariationRef = typeVariationRef
return m
}

func (m *IntervalDayType) GetPrecisionProtoVal() int32 {
return m.Precision.ToProtoVal()
}

func (*IntervalDayType) isRootRef() {}
func (m *IntervalDayType) WithNullability(n Nullability) Type {
m.Nullability = n
return m
}

func (m *IntervalDayType) WithPrecision(precision TimePrecision) *IntervalDayType {
m.Precision = precision
return m
}

func (m *IntervalDayType) GetType() Type { return m }
func (m *IntervalDayType) GetNullability() Nullability { return m.Nullability }
func (m *IntervalDayType) GetTypeVariationReference() uint32 { return m.TypeVariationRef }
func (m *IntervalDayType) Equals(rhs Type) bool {
if o, ok := rhs.(*IntervalDayType); ok {
return *o == *m
}
return false
}

func (m *IntervalDayType) ToProtoFuncArg() *proto.FunctionArgument {
return &proto.FunctionArgument{
ArgType: &proto.FunctionArgument_Type{Type: m.ToProto()},
}
}

func (m *IntervalDayType) ToProto() *proto.Type {
precisionVal := m.Precision.ToProtoVal()
return &proto.Type{Kind: &proto.Type_IntervalDay_{
IntervalDay: &proto.Type_IntervalDay{
Precision: &precisionVal,
Nullability: m.Nullability,
TypeVariationReference: m.TypeVariationRef}}}
}

func (*IntervalDayType) ShortString() string { return "iday" }
func (m *IntervalDayType) String() string {
return fmt.Sprintf("interval_day%s<%d>", strNullable(m),
m.Precision.ToProtoVal())
}

func (m *IntervalDayType) ParameterString() string {
return fmt.Sprintf("%d", m.Precision.ToProtoVal())
}

func (s *IntervalDayType) BaseString() string {
return "interval_day"
}

func (m *IntervalDayType) GetPrecision() TimePrecision {
return m.Precision
}
56 changes: 56 additions & 0 deletions types/interval_day_type_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package types

import (
"fmt"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
"github.com/substrait-io/substrait-go/proto"
"google.golang.org/protobuf/testing/protocmp"
)

func TestIntervalDayType(t *testing.T) {
allPossibleTimePrecision := []TimePrecision{PrecisionSeconds, PrecisionDeciSeconds, PrecisionCentiSeconds, PrecisionMilliSeconds,
PrecisionEMinus4Seconds, PrecisionEMinus5Seconds, PrecisionMicroSeconds, PrecisionEMinus7Seconds, PrecisionEMinus8Seconds, PrecisionNanoSeconds}
allPossibleNullability := []Nullability{NullabilityUnspecified, NullabilityNullable, NullabilityRequired}

for _, precision := range allPossibleTimePrecision {
for _, nullability := range allPossibleNullability {
expectedIntervalDayType := &IntervalDayType{Precision: precision, Nullability: nullability}
expectedFormatString := fmt.Sprintf("%s<%d>", strNullable(expectedIntervalDayType), precision.ToProtoVal())
// verify IntervalDayType
createdIntervalDayTypeIfc := (&IntervalDayType{Precision: precision}).WithNullability(nullability)
createdIntervalDayType := createdIntervalDayTypeIfc.(*IntervalDayType)
assert.True(t, createdIntervalDayType.Equals(expectedIntervalDayType))
assert.Equal(t, expectedProtoValMap[precision], createdIntervalDayType.GetPrecisionProtoVal())
assert.Equal(t, nullability, createdIntervalDayType.GetNullability())
assert.Zero(t, createdIntervalDayType.GetTypeVariationReference())
assert.Equal(t, fmt.Sprintf("interval_day%s", expectedFormatString), createdIntervalDayType.String())
assert.Equal(t, "iday", createdIntervalDayType.ShortString())
assertIntervalDayTypeProto(t, precision, nullability, createdIntervalDayType)
}
}
}

func assertIntervalDayTypeProto(t *testing.T, expectedPrecision TimePrecision, expectedNullability Nullability,
toVerifyType *IntervalDayType) {

expectedPrecisionProtoVal := expectedPrecision.ToProtoVal()
expectedTypeProto := &proto.Type{Kind: &proto.Type_IntervalDay_{
IntervalDay: &proto.Type_IntervalDay{
Precision: &expectedPrecisionProtoVal,
Nullability: expectedNullability,
},
}}
if diff := cmp.Diff(toVerifyType.ToProto(), expectedTypeProto, protocmp.Transform()); diff != "" {
t.Errorf("IntervalDayType proto didn't match, diff:\n%v", diff)
}

expectedFuncArgProto := &proto.FunctionArgument{ArgType: &proto.FunctionArgument_Type{
Type: expectedTypeProto,
}}
if diff := cmp.Diff(toVerifyType.ToProtoFuncArg(), expectedFuncArgProto, protocmp.Transform()); diff != "" {
t.Errorf("IntervalDayType func arg proto didn't match, diff:\n%v", diff)
}
}
10 changes: 10 additions & 0 deletions types/parser/type_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,14 @@ func (p *lengthType) RetType() (types.Type, error) {
return nil, substraitgo.ErrNotImplemented
}

if types.TypeName(p.TypeName) == types.TypeNameIntervalDay {
precision, err := types.ProtoToTimePrecision(lit.Value)
if err != nil {
return nil, err
}
return (&types.IntervalDayType{}).WithPrecision(precision).WithNullability(n), nil
}

typ, err := types.FixedTypeNameToType(types.TypeName(p.TypeName))
if err != nil {
return nil, err
Expand Down Expand Up @@ -306,6 +314,8 @@ func getParameterizedTypeSingleParam(typeName string, leafParam integer_paramete
return &types.ParameterizedPrecisionTimestampType{IntegerOption: leafParam, Nullability: n}, nil
case types.TypeNamePrecisionTimestampTz:
return &types.ParameterizedPrecisionTimestampTzType{IntegerOption: leafParam, Nullability: n}, nil
case types.TypeNameIntervalDay:
return &types.ParameterizedIntervalDayType{IntegerOption: leafParam, Nullability: n}, nil
default:
return nil, substraitgo.ErrNotImplemented
}
Expand Down
3 changes: 2 additions & 1 deletion types/parser/type_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func TestParser(t *testing.T) {
{"struct<list?<decimal<P,S>>, i16>", "struct<list?<decimal<P,S>>, i16>", "struct", &types.ParameterizedStructType{Types: []types.FuncDefArgType{&types.ParameterizedListType{Type: &types.ParameterizedDecimalType{Precision: parameterLeaf_P, Scale: parameterLeaf_S, Nullability: types.NullabilityRequired}, Nullability: types.NullabilityNullable}, &types.Int16Type{Nullability: types.NullabilityRequired}}, Nullability: types.NullabilityRequired}},
{"map<decimal<P,S>, i16>", "map<decimal<P,S>, i16>", "map", &types.ParameterizedMapType{Key: &types.ParameterizedDecimalType{Precision: parameterLeaf_P, Scale: parameterLeaf_S, Nullability: types.NullabilityRequired}, Value: &types.Int16Type{Nullability: types.NullabilityRequired}, Nullability: types.NullabilityRequired}},
{"precision_timestamp_tz?<L1>", "precision_timestamp_tz?<L1>", "pretstz", &types.ParameterizedPrecisionTimestampTzType{IntegerOption: parameterLeaf_L1}},
{"interval_day<5>", "interval_day<5>", "iday", &types.ParameterizedIntervalDayType{IntegerOption: concreteLeaf_5}},
}

p, err := parser.New()
Expand Down Expand Up @@ -80,7 +81,7 @@ func TestParserRetType(t *testing.T) {
shortName string
expectedTyp types.Type
}{
{"interval_day?<1>", "interval_day?<1>", "iday", &types.IntervalDayType{Length: 1, Nullability: types.NullabilityNullable}},
{"interval_day?<1>", "interval_day?<1>", "iday", &types.IntervalDayType{Precision: 1, Nullability: types.NullabilityNullable}},
}

p, err := parser.New()
Expand Down
9 changes: 4 additions & 5 deletions types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ var fixedTypeNameMap = map[TypeName]FixedType{
TypeNameFixedBinary: &FixedBinaryType{},
TypeNameFixedChar: &FixedCharType{},
TypeNameVarChar: &VarCharType{},
TypeNameIntervalDay: &IntervalDayType{},
}

var shortTypeNames = map[TypeName]string{
Expand Down Expand Up @@ -123,6 +122,7 @@ func GetTypeNameToTypeMap() map[string]Type {
typeMap[string(k)] = v
}
typeMap[string(TypeNameDecimal)] = &DecimalType{}
typeMap[string(TypeNameIntervalDay)] = &IntervalDayType{}
return typeMap
}

Expand Down Expand Up @@ -264,7 +264,7 @@ func TypeFromProto(t *proto.Type) Type {
return &IntervalDayType{
Nullability: t.IntervalDay.Nullability,
TypeVariationRef: t.IntervalDay.TypeVariationReference,
Length: precision.ToProtoVal(),
Precision: precision,
}
case *proto.Type_TimestampTz:
return &TimestampTzType{
Expand Down Expand Up @@ -501,7 +501,7 @@ func TypeToProto(t Type) *proto.Type {
Nullability: t.Nullability,
TypeVariationReference: t.TypeVariationRef}}}
case *IntervalDayType:
precision := t.Length
precision := t.Precision.ToProtoVal()
return &proto.Type{Kind: &proto.Type_IntervalDay_{
IntervalDay: &proto.Type_IntervalDay{
Precision: &precision,
Expand Down Expand Up @@ -704,7 +704,6 @@ type (
FixedCharType = FixedLenType[FixedChar]
VarCharType = FixedLenType[VarChar]
FixedBinaryType = FixedLenType[FixedBinary]
IntervalDayType = FixedLenType[IntervalDayToSecond]
ParameterizedVarCharType = parameterizedTypeSingleIntegerParam[*VarCharType]
ParameterizedFixedCharType = parameterizedTypeSingleIntegerParam[*FixedCharType]
ParameterizedFixedBinaryType = parameterizedTypeSingleIntegerParam[*FixedBinaryType]
Expand All @@ -715,7 +714,7 @@ type (

// FixedLenType is any of the types which also need to track their specific
// length as they have a fixed length.
type FixedLenType[T FixedChar | VarChar | FixedBinary | IntervalDayToSecond] struct {
type FixedLenType[T FixedChar | VarChar | FixedBinary] struct {
Nullability Nullability
TypeVariationRef uint32
Length int32
Expand Down
17 changes: 9 additions & 8 deletions types/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestTypeToString(t *testing.T) {
{&TimeType{}, "time", "time"},
{&TimestampTzType{}, "timestamp_tz", "tstz"},
{&IntervalYearType{}, "interval_year", "iyear"},
{&IntervalDayType{Length: 5}, "interval_day<5>", "iday"},
{&IntervalDayType{Precision: 5}, "interval_day<5>", "iday"},
{&UUIDType{Nullability: NullabilityNullable}, "uuid?", "uuid"},
{&FixedBinaryType{Length: 10}, "fixedbinary<10>", "fbin"},
{&FixedCharType{Length: 5}, "char<5>", "fchar"},
Expand Down Expand Up @@ -77,13 +77,13 @@ func TestTypeRoundtrip(t *testing.T) {
&TimestampType{Nullability: n},
&TimestampTzType{Nullability: n},
&IntervalYearType{Nullability: n},
&IntervalDayType{Nullability: n},
&UUIDType{Nullability: n},
&FixedCharType{Nullability: n, Length: 25},
&VarCharType{Nullability: n, Length: 35},
&FixedBinaryType{Nullability: n, Length: 45},
&IntervalDayType{Nullability: n, Length: 5},
&IntervalDayType{Nullability: n},
&IntervalDayType{Nullability: n, Precision: 5},
&IntervalDayType{Nullability: n, Precision: 0},

&DecimalType{Nullability: n, Precision: 34, Scale: 3},
&MapType{Nullability: n, Key: &Int8Type{}, Value: &Int16Type{Nullability: n}},
&ListType{Nullability: n, Type: &TimeType{Nullability: n}},
Expand All @@ -95,7 +95,8 @@ func TestTypeRoundtrip(t *testing.T) {
for _, tt := range tests {
t.Run(tt.String(), func(t *testing.T) {
converted := TypeToProto(tt)
assert.True(t, tt.Equals(TypeFromProto(converted)))
convertedType := TypeFromProto(converted)
assert.True(t, tt.Equals(convertedType))
})
}
})
Expand Down Expand Up @@ -267,7 +268,7 @@ func TestMatchParameterizeConcreteTypeResultMatch(t *testing.T) {
fixedCharLen5 := &FixedCharType{Length: 5}
varCharLen5 := &VarCharType{Length: 5}
fixedBinaryLen5 := &FixedBinaryType{Length: 5}
intervalDayLen5 := &IntervalDayType{Length: 5}
intervalDayLen5 := &IntervalDayType{Precision: 5}

concreteInt38 := integer_parameters.NewConcreteIntParam(38)
concreteInt2 := integer_parameters.NewConcreteIntParam(2)
Expand Down Expand Up @@ -309,7 +310,7 @@ func TestMatchParameterizeConcreteTypeResultMismatch(t *testing.T) {
fixedCharLen6 := &FixedCharType{Length: 6}
varCharLen6 := &VarCharType{Length: 6}
fixedBinaryLen6 := &FixedBinaryType{Length: 6}
intervalDayLen6 := &IntervalDayType{Length: 6}
intervalDayLen6 := &IntervalDayType{Precision: 6}

concreteInt38 := integer_parameters.NewConcreteIntParam(38)
concreteInt2 := integer_parameters.NewConcreteIntParam(2)
Expand Down Expand Up @@ -354,7 +355,7 @@ func TestMatchParameterizedNonNestedTypeResultMatch(t *testing.T) {
paramPrecisionTimeStampTz := &ParameterizedPrecisionTimestampTzType{IntegerOption: intParamLen}
argDecimalType := &DecimalType{Precision: 38, Scale: 2}
paramDecimalType := &ParameterizedDecimalType{Precision: integer_parameters.NewVariableIntParam("P"), Scale: integer_parameters.NewVariableIntParam("S")}
argIntervalDayType := &IntervalDayType{Length: 5}
argIntervalDayType := &IntervalDayType{Precision: 5}
paramIntervalDayType := &ParameterizedIntervalDayType{IntegerOption: intParamLen}

for _, td := range []struct {
Expand Down

0 comments on commit 65f92ce

Please sign in to comment.