Eliaaazzz commented on issue #39684:
URL: https://github.com/apache/beam/issues/39684#issuecomment-5715032275

   Hi Yi, I went through the Go schema code against the six gaps above and they 
all check out on current master. Two extra findings on the way: the Go row 
coder encodes INT16 as a varint while Java and Python use 2 byte big endian, 
and the Go standard coder regression test skipped every logical type row case 
with one "logical" filter, so the 7 yaml cases can serve as acceptance tests.
   
   I split the work into a series of PRs, each one commit stacked on the 
previous one:
   
   1. #40152 fixes the INT16 row encoding. This is the separate bug #40151, and 
timestamp:v1 below precision 5 needs it.
   2. #40153 plumbs argument_type and argument through the schema proto 
conversion and keys the registry by URN and argument.
   3. #40154 adds a single registration call for portable logical types, 
schema.RegisterLogicalTypeConversion with its parameterized and coder variants, 
and the standard date and micros_instant types.
   4. #40155 adds millis_instant and decimal, whose wire encodings differ from 
their representation types.
   5. #40156 maps fixed_char, var_char, fixed_bytes and var_bytes to string and 
[]byte as pass through types.
   6. #40157 adds timestamp:v1 as TimestampMillis, TimestampMicros and 
TimestampNanos.
   7. #40158 updates the programming guide.
   
   With the last one, every logical type row case in standard_coders.yaml runs 
in the Go regression test.
   
   Two decisions I made that are easy to change if you prefer otherwise. First, 
time.Time keeps its current Go specific encoding and the standard types are 
separate Go types, so nothing existing breaks. Second, because the Go row coder 
is built from the Go type alone, each URN and argument value maps to one named 
Go type, so timestamp:v1 covers precisions 3, 6 and 9, and the string and bytes 
types do not enforce their length argument.
   
   If the other assignees have work in progress on any part of this, I am happy 
to rebase or drop the overlapping PR.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to