Skip to content

feat(go): add missing type resolver for uint{16,32,64}slice#3311

Merged
chaokunyang merged 1 commit intoapache:mainfrom
BrianLii:issue-uint-slice
Feb 9, 2026
Merged

feat(go): add missing type resolver for uint{16,32,64}slice#3311
chaokunyang merged 1 commit intoapache:mainfrom
BrianLii:issue-uint-slice

Conversation

@BrianLii
Copy link
Contributor

@BrianLii BrianLii commented Feb 9, 2026

Why?

The uint serializer and specific uint16, uint32, uint64 slice serializers were implemented but missing from the TypeResolver registration and dispatch logic. This caused not supported errors when using these types.

What does this PR do?

  1. Type Registration:

    • Registers uint16Type, uint32Type, and uint64Type in newTypeResolver initialization to ensure decodeType can correctly resolve these type strings.
  2. Serializer Dispatch:

    • Updates createSerializer in type_resolver.go to correctly dispatch to the existing optimized uint16SliceSerializer, uint32SliceSerializer, and uint64SliceSerializer for the corresponding slice types.
  3. Tests:

    • Adds test cases in TestTypeResolver (type_test.go) to verify that []uint16, []uint32, and []uint64 are correctly resolved and serialized.

Related issues

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

@BrianLii BrianLii marked this pull request as ready for review February 9, 2026 03:34
@BrianLii BrianLii requested a review from chaokunyang as a code owner February 9, 2026 03:34
"[]map[string][]map[string]*interface {}"},
{reflect.TypeOf((*A)(nil)), "*@example.A"},
{reflect.TypeOf((*A)(nil)).Elem(), "@example.A"},
{reflect.TypeOf([]uint16{}), "[]uint16"},
Copy link
Collaborator

@chaokunyang chaokunyang Feb 9, 2026

Choose a reason for hiding this comment

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

Could you also add a serialization test in go/fory/slice_primitive_test.go ?

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'd be happy to. I'll add the tests for int/uint slice in a seperate PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add the tests in this PR? The fix should have coresponing tests in same PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review. Because this PR only fixes the type_resolver, shouldn't the tests in go/fory/type_test.go be enough? I can also include the test for int8, int16, int32, int64 in type_test.go

Copy link
Collaborator

@chaokunyang chaokunyang left a comment

Choose a reason for hiding this comment

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

LGTM

@chaokunyang chaokunyang merged commit ef25ea2 into apache:main Feb 9, 2026
57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants