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

Register Go int as long in type resolver #423

Merged
merged 3 commits into from
Jul 31, 2024
Merged

Register Go int as long in type resolver #423

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90e99fa
register Go int as long in type resolver
lovromazgon Jul 30, 2024
22e6596
add union resolver tests
lovromazgon Jul 30, 2024
14fdcf0
test cases for logical types
lovromazgon Jul 30, 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
91 changes: 91 additions & 0 deletions encoder_union_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,3 +550,94 @@ func TestEncoder_UnionInterfaceNotInSchema(t *testing.T) {

assert.Error(t, err)
}

func TestEncoder_UnionResolver(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't find any tests testing this behavior so I added it, specifically the test case Go int as Avro long tests the line added in this PR. It's written in a different style than the other tests in this repo, if you prefer I can split it up into separate tests.

testCases := []struct {
name string
schema string
value any
want []byte
}{
{
name: "Go int8 as Avro int",
schema: `["null", "int"]`,
value: int8(27),
want: []byte{0x2, 0x36},
},
{
name: "Go int16 as Avro int",
schema: `["null", "int"]`,
value: int16(27),
want: []byte{0x2, 0x36},
},
{
name: "Go int32 as Avro int",
schema: `["null", "int"]`,
value: int32(27),
want: []byte{0x2, 0x36},
},
{
name: "Go int as Avro int",
schema: `["null", "int"]`,
value: int(27),
want: []byte{0x2, 0x36},
},
{
name: "Go int as Avro long",
schema: `["null", "long"]`,
value: int(2147483648),
want: []byte{0x2, 0x80, 0x80, 0x80, 0x80, 0x10},
},
{
name: "Go int64 as Avro long",
schema: `["null", "long"]`,
value: int64(2147483648),
want: []byte{0x2, 0x80, 0x80, 0x80, 0x80, 0x10},
},
{
name: "Go float32 as Avro float",
schema: `["null", "float"]`,
value: float32(1.15),
want: []byte{0x2, 0x33, 0x33, 0x93, 0x3F},
},
{
name: "Go float64 as Avro float",
schema: `["null", "double"]`,
value: float64(1.15),
want: []byte{0x2, 0x66, 0x66, 0x66, 0x66, 0x66, 0x66, 0xF2, 0x3F},
},
{
name: "Go string as Avro string",
schema: `["null", "string"]`,
value: "foo",
want: []byte{0x2, 0x06, 0x66, 0x6F, 0x6F},
},
{
name: "Go []byte as Avro bytes",
schema: `["null", "bytes"]`,
value: []byte{0xEC, 0xAB, 0x44, 0x00},
want: []byte{0x2, 0x08, 0xEC, 0xAB, 0x44, 0x00},
},
{
name: "Go bool as Avro boolean",
schema: `["null", "boolean"]`,
value: true,
want: []byte{0x2, 0x01},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
defer ConfigTeardown()

buf := bytes.NewBuffer([]byte{})
enc, err := avro.NewEncoder(tc.schema, buf)
require.NoError(t, err)

err = enc.Encode(tc.value)

require.NoError(t, err)
assert.Equal(t, tc.want, buf.Bytes())
})
}
}
1 change: 1 addition & 0 deletions resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func NewTypeResolver() *TypeResolver {
r.Register(string(Int), int16(0))
r.Register(string(Int), int32(0))
r.Register(string(Int), int(0))
r.Register(string(Long), int(0))
Copy link
Member

Choose a reason for hiding this comment

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

Should this only be for 64 but systems?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, let me explain.

In case of encoding, you can encode int32 and int64 as long, both should be fine. And this is enabled now because TypeResolver.Name(int) will return long as a possible type for encoding int.

Now, in the case of decoding it's only safe to decode long into int64. This is till the case though, because in the next line we register int64 as long, so TypeResolver.Type("long") will still return int64, as it was before.

Copy link
Member

Choose a reason for hiding this comment

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

Fair point

r.Register(string(Long), int64(0))
r.Register(string(Float), float32(0))
r.Register(string(Double), float64(0))
Expand Down
Loading