Repository navigation
Conversation
402fcc7 to
f518a7c
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #40153 +/- ##
=============================================
- Coverage 56.16% 28.59% -27.57%
=============================================
Files 1124 380 -744
Lines 178336 70970 -107366
Branches 1489 0 -1489
=============================================
- Hits 100170 20297 -79873
+ Misses 75645 48835 -26810
+ Partials 2521 1838 -683
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Assigning reviewers: R: @lostluck for label go. This pull request likely touches a core component ("core" label). Please review with scrutiny. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
Reminder, please take a look at this pr: @lostluck |
f518a7c to
b979fd6
Compare
The reflection based row coder used the varint encoding for int16 and uint16 fields. Java and Python encode the INT16 schema type as 2 big endian bytes, so rows with such fields could not be exchanged with the other SDKs. Fixes apache#40151. Part of apache#39684.
Logical types registered with an argument now emit argument_type and argument in their FieldType, and schemas from other SDKs are matched on the URN and the argument value. A logical type registered without an argument still matches any argument, so pass through types keep working. Top level logical types carry the argument in a schema option. Part of apache#39684.
b979fd6 to
d4c5f0b
Compare
|
Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment R: @jrmccluskey for label go. This pull request likely touches a core component ("core" label). Please review with scrutiny. Available commands:
|
restore whitespace
Part of #39684. Depends on #40152 (the first commit); review the last commit.
schema.LogicalTypehadargTandargVfields that nothing populated,reflectTypeToFieldTypeleftargument_typeandargumentunset (theTODO(BEAM-9615)in schema.go), andfieldTypeToReflectTypeignored them. This PR wires the argument through:schema.ToLogicalTypeWithArgumentcreates a logical type with an argument. The argument must be a value of a built in Go type with an atomic schema type other than[]byte, which covers the INT32 arguments of the standard parameterized logical types.beam:logical_type:var_char:v1can be registered once for any length. Go ignores non atomic arguments. The BYTE schema type is signed, so auint8argument is converted throughint8, and a floating point NaN is rejected at registration because it never compares equal.FromTypeemitsargument_typeandargumentfor parameterized logical types, and top level parameterized logical types carry the argument in abeam:schema:go:logical_argument:v1schema option soToTypecan recover the Go type.FromTypereturns an error instead of panicking with a nil pointer when a struct kind logical type with a non row storage type is used as a top level type.fieldvalue.goconverts atomic values betweenreflect.ValueandFieldType/FieldValue, and rejects BYTE and INT16 values outside their range.Tests: schema conversion cases for a parameterized field and a parameterized top level type,
TestToType_LogicalTypeArgumentsfor matching, missing, unknown, ignored and non atomic arguments,TestLogicalTypeArgumentPrecedencefor two arguments and a fallback under one URN (fields and top level), registration validation, and round trip tests for the atomic value conversion. Tests also cover out of range BYTE and INT16 values, and unregistered top level logical types after RegisterType, for value, pointer and defined pointer Go types.Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.