Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update proto definition to v0.55.0 #48

Merged
merged 2 commits into from
Aug 20, 2024
Merged

Update proto definition to v0.55.0 #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de668ec
Update proto definition to v0.55.0
anshuldata Aug 19, 2024
eeed6a1
Added other join type in switch (kept unimplemented)
anshuldata Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions doc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// Package substraitgo contains the experimental go bindings for substrait
// (https://substrait.io).
//
// Current generated proto substrait version: v0.53.0
// Current generated proto substrait version: v0.55.0
package substraitgo

//go:generate buf generate https://github.com/substrait-io/substrait.git#tag=v0.53.0
//go:generate buf generate https://github.com/substrait-io/substrait.git#tag=v0.55.0
8 changes: 4 additions & 4 deletions plan/plan_builder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,12 +619,12 @@ func TestJoinRelOutputRecordTypes(t *testing.T) {
recordString string
}{
{"JOIN_TYPE_INNER", plan.JoinTypeInner, []string{"a", "b", "c", "d"}, "NSTRUCT<a: string, b: fp32, c: i32, d: boolean>"},
{"JOIN_TYPE_SEMI", plan.JoinTypeSemi, []string{"a", "b"}, "NSTRUCT<a: string, b: fp32>"},
{"JOIN_TYPE_LEFT_SEMI", plan.JoinTypeLeftSemi, []string{"a", "b"}, "NSTRUCT<a: string, b: fp32>"},
{"JOIN_TYPE_OUTER", plan.JoinTypeOuter, []string{"a", "b", "c", "d"}, "NSTRUCT<a: string?, b: fp32?, c: i32?, d: boolean?>"},
{"JOIN_TYPE_LEFT", plan.JoinTypeLeft, []string{"a", "b", "c", "d"}, "NSTRUCT<a: string, b: fp32, c: i32?, d: boolean?>"},
{"JOIN_TYPE_RIGHT", plan.JoinTypeRight, []string{"a", "b", "c", "d"}, "NSTRUCT<a: string?, b: fp32?, c: i32, d: boolean>"},
{"JOIN_TYPE_ANTI", plan.JoinTypeAnti, []string{"a", "b"}, "NSTRUCT<a: string, b: fp32>"},
{"JOIN_TYPE_SINGLE", plan.JoinTypeSingle, []string{"a", "b", "c", "d"}, "NSTRUCT<a: string, b: fp32, c: i32?, d: boolean?>"},
{"JOIN_TYPE_LEFT_ANTI", plan.JoinTypeLeftAnti, []string{"a", "b"}, "NSTRUCT<a: string, b: fp32>"},
{"JOIN_TYPE_LEFT_SINGLE", plan.JoinTypeLeftSingle, []string{"a", "b", "c", "d"}, "NSTRUCT<a: string, b: fp32, c: i32?, d: boolean?>"},
}

for _, tt := range tests {
Expand Down Expand Up @@ -764,7 +764,7 @@ func TestJoinRelationError(t *testing.T) {
assert.ErrorIs(t, err, substraitgo.ErrInvalidRel)
assert.ErrorContains(t, err, "output mapping index out of range")

_, err = b.JoinRemap(left, right, goodcond, plan.JoinTypeAnti, []int32{2})
_, err = b.JoinRemap(left, right, goodcond, plan.JoinTypeLeftAnti, []int32{2})
assert.ErrorIs(t, err, substraitgo.ErrInvalidRel)
assert.ErrorContains(t, err, "output mapping index out of range")

Expand Down
19 changes: 13 additions & 6 deletions plan/relations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
package plan

import (
"fmt"

substraitgo "github.com/substrait-io/substrait-go"
"github.com/substrait-io/substrait-go/expr"
"github.com/substrait-io/substrait-go/extensions"
Expand Down Expand Up @@ -585,9 +587,12 @@
JoinTypeOuter = proto.JoinRel_JOIN_TYPE_OUTER
JoinTypeLeft = proto.JoinRel_JOIN_TYPE_LEFT
JoinTypeRight = proto.JoinRel_JOIN_TYPE_RIGHT
JoinTypeSemi = proto.JoinRel_JOIN_TYPE_SEMI
JoinTypeAnti = proto.JoinRel_JOIN_TYPE_ANTI
JoinTypeSingle = proto.JoinRel_JOIN_TYPE_SINGLE
JoinTypeLeftSemi = proto.JoinRel_JOIN_TYPE_LEFT_SEMI
JoinTypeLeftAnti = proto.JoinRel_JOIN_TYPE_LEFT_ANTI
JoinTypeLeftSingle = proto.JoinRel_JOIN_TYPE_LEFT_SINGLE
JoinTypeRightSemi = proto.JoinRel_JOIN_TYPE_RIGHT_SEMI
JoinTypeRightAnti = proto.JoinRel_JOIN_TYPE_RIGHT_ANTI
JoinTypeRightSingle = proto.JoinRel_JOIN_TYPE_RIGHT_SINGLE
)

// JoinRel is a binary Join relational operator representing left-join-right,
Expand All @@ -607,14 +612,14 @@
switch j.joinType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you going to need JoinTypeRightSingle and JoinTypeRightAnti for all of the options to be handled without a default clause?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR I intended to keep for proto change so I planned to handle them in a separate PR. I updated PR to add missing ones to indicate unsupported. Kindly check

case JoinTypeInner:
return j.JoinedRecordType()
case JoinTypeSemi:
case JoinTypeLeftSemi:
return j.left.Remap(j.left.RecordType())
case JoinTypeOuter:
typeList = j.JoinedRecordType().Types
for i, t := range typeList {
typeList[i] = t.WithNullability(types.NullabilityNullable)
}
case JoinTypeLeft, JoinTypeSingle:
case JoinTypeLeft, JoinTypeLeftSingle:
left := j.left.Remap(j.left.RecordType())
right := j.right.Remap(j.right.RecordType())
typeList = make([]types.Type, 0, len(left.Types)+len(right.Types))
Expand All @@ -630,8 +635,10 @@
typeList = append(typeList, l.WithNullability(types.NullabilityNullable))
}
typeList = append(typeList, right.Types...)
case JoinTypeAnti:
case JoinTypeLeftAnti:
typeList = j.left.RecordType().Types
case JoinTypeRightSemi, JoinTypeRightAnti, JoinTypeRightSingle:
panic(fmt.Sprintf("join type: %v not supported", j.joinType))

Check warning on line 641 in plan/relations.go

View check run for this annotation

Codecov / codecov/patch

plan/relations.go#L640-L641 

Added lines #L640 - L641 were not covered by tests
}

return types.StructType{
Expand Down
Loading
Loading