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.

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

Add Codec.ScanBinary #271

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add Codec.ScanBinary #271

wants to merge 3 commits into from

Conversation

SpencerC
Copy link

@SpencerC SpencerC commented Jun 7, 2023

Motivation

Currently, developers calling the Codec.Native functions need to write glue code to traverse the returned map[string]interface{}s and convert the values. While this approach makes sense for maintaining the symmetry between the encoder input and the decoder output and for its reflection of the underlying architecture of the format, it creates some unnecessary friction in applications where the goal is simply to get data into simple, native types.

Background

I recently created a Go SDK for DataStax Astra which provides methods for developers to work with returned data without worrying about the underlying API. It implements a Scan function that mirrors the functionality of the builtin database/sql#Rows.Scan function. I think a similar approach might be useful here.

convertAssign: datastax-ext/astra-go-sdk#15
Row.Scan: datastax-ext/astra-go-sdk#16

Future Work

  • Implement scan logic for maps and slices.
  • Create testBinaryScan function and add more test cases.
  • Create a Codec function to expose the field name order. Unlike the case where Scan calls follow database queries, schemas may be loaded from outside the code and the field order may not be visible to developers. Giving developers access to the field name order would allow them to create runtime mappings between fields and destinations (example)

@SpencerC
Copy link
Author

SpencerC commented Jun 7, 2023

It turns out this approach is also more performant. I'm guessing because it requires fewer dynamic allocations? Benchmark results:

goos: darwin
goarch: arm64
pkg: github.com/linkedin/goavro/v2
BenchmarkNewCodecUsingV2-10                        98484             10709 ns/op
BenchmarkNativeFromAvroUsingV2-10                    513           2330489 ns/op
BenchmarkBinaryFromNativeUsingV2-10                 1555            747692 ns/op
BenchmarkNativeFromBinaryUsingV2-10                  582           2046254 ns/op
BenchmarkTextualFromNativeUsingV2-10                 262           4534945 ns/op
BenchmarkNativeFromTextualUsingV2-10                 240           4984041 ns/op
BenchmarkScanBinaryUsingV2-10                        618           1941487 ns/op
PASS
ok      github.com/linkedin/goavro/v2   10.183s

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.

None yet

1 participant