-
Notifications
You must be signed in to change notification settings - Fork 592
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
feat(sink): ProtoEncoder and AvroEncoder #12425
Conversation
0c75ec6
to
c4930c2
Compare
2352aac
to
7062fdb
Compare
Codecov Report
@@ Coverage Diff @@
## main #12425 +/- ##
==========================================
+ Coverage 69.26% 69.38% +0.11%
==========================================
Files 1480 1482 +2
Lines 243571 244638 +1067
==========================================
+ Hits 168721 169742 +1021
- Misses 74850 74896 +46
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
ca3d74b
to
8d57d0f
Compare
8d57d0f
to
2717d5b
Compare
2717d5b
to
268e581
Compare
Are there still concerns before we merge this? |
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
For both avro and proto, we validate between RisingWave and Encoder schema during
Encoder::new
. The duplicated logic (mostly m x n match between data type combinations) between validate-with-type-only and encode-with-actual-data is shared with the help ofMaybeData
andencode_field
. The call graph of core functions is as follows (dot-src):There are some encoding specific complexities:
["null", T]
(union of null and T). The upstream library is also buggy in some aspects, requiring more details we need to handle properly on our own (seetest_encode_avro_lib_bug
).We will extend to support more data type combinations gradually. For any unsupported RisingWave type, the user can trivially
::varchar
in RisingWave as a workaround.Note this is not usable by user yet. There is one last step to parse
format_desc.options
inSinkFormatterImpl::new
to retrieveMessageDescriptor
/AvroSchema
and then construct these encoders.Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.