You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fory.apache.org by "lzaeh (via GitHub)" <gi...@apache.org> on 2025/10/13 10:58:53 UTC
[PR] fix(go): adjustments to the sliceSerializer [fory]
lzaeh opened a new pull request, #2760:
URL: https://github.com/apache/fory/pull/2760
<!--
**Thanks for contributing to Apache Fory™.**
**If this is your first time opening a PR on fory, you can refer to [CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).**
Contribution Checklist
- The **Apache Fory™** community has requirements on the naming of pr titles. You can also find instructions in [CONTRIBUTING.md](https://github.com/apache/fory/blob/main/CONTRIBUTING.md).
- Apache Fory™ has a strong focus on performance. If the PR you submit will have an impact on performance, please benchmark it first and provide the benchmark result here.
-->
## Why?
Previously, my understanding of the type system was insufficient, which led to confusion between array and slice usage. As a result, the deserialization step of the slice serializer did not work properly in cross-language scenarios due to missing alignment rules (for example, when the types were structurally identical and should have taken the fast path, the previous version failed to correctly interpret the byte stream).
<!-- Describe the purpose of this PR. -->
## What does this PR do?
Serializer-related changes:
- Adjusted the slice serializer to better align serialization and deserialization rules.
- Standardized the type system to ensure that the slice serializer is consistently used where appropriate, eliminating mixed usage with array serializers.
- Moved the serializer for primitive type arrays into ```primitive_array.go```. If you think it’s not appropriate, I can revert the change.
Test-related changes:
- Re-enabled previously commented-out test cases in commonSlice/Array.
- Moved the [n]String test out of commonArray. Although it can be correctly deserialized into []String, the current test logic does not treat [n]String and []String as equivalent.
- Updated several primitive array tests in TestXLangSerializer to use array types instead of slices.
- Modified the check function in TestXLangSerializer to deserialize using the precise reflected type instead of interface{}.
Rationale:
- For example, []interface{}{"a", "b", "c"} and []string{"a", "b", "c"} are considered the same type and written via the fast path, resulting in identical byte sequences.
- However, during deserialization in Go, the default behavior is to use []interface{} unless the target type is explicitly provided.(Just my opinion)
- This means that if the test expects []string{}, it would fail. While we could force construction of a []T{} when the same type is detected, if the test’s starting point is []interface{}, it would still fail. Therefore, the deserialization target type is now explicitly matched via reflection.
<!-- Describe the details of this PR. -->
Known Issues
- After pulling the latest code (which includes modifications related to field order hashing in structs), the complex struct tests that passed 10 mins ago now fail due to Python-side deserialization errors.
- A. Direct testing shows identical hashes, but deserialization in Python triggers an “invalid ID (id = 0)” error in the listSerializer—possibly related to changes in this PR.
- B. Simplifying the struct on both the Python and Go sides (by removing fields) caused the hash values to differ.
I will follow up on this issue to determine whether the cause lies in the list serializer or incomplete handling of struct field order consistency.
- Cross-language list serialization tests (serializing in one language, deserializing in another—Python ↔ Go, Java ↔ Go) revealed that Go’s int and float64 types cannot be correctly deserialized in the other languages.
- All other basic types work fine, and Go can correctly deserialize bytes from both Python and Java.
- When encountering floating-point values, Go must use float32 to deserialize successfully.
- The slice serializer currently mimics Python’s fast-path logic, but the matching rules for integer and floating-point types are somewhat ambiguous. My understanding here may be incomplete, and this part likely needs further refinement.
## Related issues
- #2633
<!--
Is there any related issue? If this PR closes them you say say fix/closes:
- #xxxx0
- #xxxx1
- Fixes #xxxx2
-->
## Does this PR introduce any user-facing change?
<!--
If any user-facing interface changes, please [open an issue](https://github.com/apache/fory/issues/new/choose) describing the need to do so and update the document if necessary.
Delete section if not applicable.
-->
- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?
## Benchmark
<!--
When the PR has an impact on performance (if you don't know whether the PR will have an impact on performance, you can submit the PR first, and if it will have impact on performance, the code reviewer will explain it), be sure to attach a benchmark data here.
Delete section if not applicable.
-->
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "lzaeh (via GitHub)" <gi...@apache.org>.
lzaeh commented on code in PR #2760:
URL: https://github.com/apache/fory/pull/2760#discussion_r2433278746
##########
go/fory/primitive_array.go:
##########
@@ -0,0 +1,450 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package fory
+
+import (
+ "fmt"
+ "reflect"
+)
+
+type boolArraySerializer struct {
+}
+
+func (s boolArraySerializer) TypeId() TypeId {
+ return BOOL_ARRAY
+}
+
+func (s boolArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s boolArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteBool(value.Index(i).Bool())
+ }
+ return nil
+}
+
+func (s boolArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(true)) // [length]bool
+ r = reflect.New(arrT).Elem()
+
+ default:
+ return fmt.Errorf("unsupported kind %v, want slice/array/interface", type_.Kind())
+ }
+
+ for i := 0; i < length; i++ {
+ b := buf.ReadBool()
+ r.Index(i).SetBool(b)
+ }
+
+ value.Set(r)
+ return nil
+}
+
+type int8ArraySerializer struct {
+}
+
+func (s int8ArraySerializer) TypeId() TypeId {
+ return INT8_ARRAY
+}
+
+func (s int8ArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s int8ArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteByte_(byte(value.Index(i).Int()))
+ }
+ return nil
+}
+
+func (s int8ArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(int8(0))) // [length]int8
Review Comment:
ok, thanks for telling me this.
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "lzaeh (via GitHub)" <gi...@apache.org>.
lzaeh commented on code in PR #2760:
URL: https://github.com/apache/fory/pull/2760#discussion_r2445520383
##########
go/fory/primitive_array.go:
##########
@@ -0,0 +1,450 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package fory
+
+import (
+ "fmt"
+ "reflect"
+)
+
+type boolArraySerializer struct {
+}
+
+func (s boolArraySerializer) TypeId() TypeId {
+ return BOOL_ARRAY
+}
+
+func (s boolArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s boolArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteBool(value.Index(i).Bool())
+ }
+ return nil
+}
+
+func (s boolArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(true)) // [length]bool
+ r = reflect.New(arrT).Elem()
+
+ default:
+ return fmt.Errorf("unsupported kind %v, want slice/array/interface", type_.Kind())
+ }
+
+ for i := 0; i < length; i++ {
+ b := buf.ReadBool()
+ r.Index(i).SetBool(b)
+ }
+
+ value.Set(r)
+ return nil
+}
+
+type int8ArraySerializer struct {
+}
+
+func (s int8ArraySerializer) TypeId() TypeId {
+ return INT8_ARRAY
+}
+
+func (s int8ArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s int8ArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteByte_(byte(value.Index(i).Int()))
+ }
+ return nil
+}
+
+func (s int8ArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(int8(0))) // [length]int8
Review Comment:
> we can't use `[n]T` for deserialization, the `n` used by user is fixed at compile-time, but the array length can be different. We should use slice `[]numeric` for primitive array type in fory type system.
>
> For golang serialization, we just don't allow `[n]T` being deserialized. Or only allow `[n]T` being serialized, but this seems useless. If you can't deserialzie it, serialize it seems unmeaningful too
Sorry for the delay—I just wrapped up my graduation stuff. If all arrays are deserialized into slices, should arrays inside structs also be deserialized as slices? If so, I think the tests will need to be updated as well. I was considering this while planning comprehensive tests for the contents of commonArray.@chaokunyang
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "lzaeh (via GitHub)" <gi...@apache.org>.
lzaeh commented on PR #2760:
URL: https://github.com/apache/fory/pull/2760#issuecomment-3407346591
> @lzaeh could you merge main branch first? there are some conflict
i will try asap tomorrow, sorry for the delay.
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "lzaeh (via GitHub)" <gi...@apache.org>.
lzaeh commented on PR #2760:
URL: https://github.com/apache/fory/pull/2760#issuecomment-3432113527
## Scope of this change
### This change removes array support only along the usage path, reflected in:
1. getTypeInfo behavior: except for []byte (which still uses its concrete type to enable zero-copy, out-of-band storage optimizations, etc.), all other []T are treated as lists.
2. Reflect checks: code paths that handled reflect.Array have been removed. Since arrays aren’t supported, they’re considered unavailable to users.
3. User-supplied arrays: if a user forces an array, per (1) the system reports an error indicating [N]T is not supported.
### What I did not change
I’m unsure whether we should purge array-related constructs from the type system itself. Fully removing them would touch a lot of code. Keeping the definitions around but unused might serve as a transitional/contingency option, even if arrays are effectively deprecated right now.
That said, if we do want to remove arrays from the type system entirely, let me know and I’ll follow up.
### Cross-language test adjustments
For TestXLangSerializer: since other languages may interact with the same unit as Python, I removed the array-related code paths on both the Go and Python sides within this test. I then added a separate test_cross_language_serializer_go case in Python’s cross-language tests so that Go still covers this part. If this setup isn’t appropriate, please let me know.
### Remaining work (Relatively higher-priority tasks, Just my opinion)
Update the struct field ordering logic accordingly. After updating the Python module, two struct tests failed due to hash mismatches. I haven’t changed the handling of complex structs, which means the code path for fields that are arrays within complex structs remains untouched—and this test includes arrays. If I tweak just one spot now, it’ll be hard to localize the issue. It’s better to make the corresponding changes after I spend time learning the new sorting/ordering optimization algorithm, so we can update everything in sync.
@chaokunyang
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2760:
URL: https://github.com/apache/fory/pull/2760#discussion_r2429681513
##########
go/fory/primitive_array.go:
##########
@@ -0,0 +1,450 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package fory
+
+import (
+ "fmt"
+ "reflect"
+)
+
+type boolArraySerializer struct {
+}
+
+func (s boolArraySerializer) TypeId() TypeId {
+ return BOOL_ARRAY
+}
+
+func (s boolArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s boolArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteBool(value.Index(i).Bool())
+ }
+ return nil
+}
+
+func (s boolArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(true)) // [length]bool
+ r = reflect.New(arrT).Elem()
+
+ default:
+ return fmt.Errorf("unsupported kind %v, want slice/array/interface", type_.Kind())
+ }
+
+ for i := 0; i < length; i++ {
+ b := buf.ReadBool()
+ r.Index(i).SetBool(b)
+ }
+
+ value.Set(r)
+ return nil
+}
+
+type int8ArraySerializer struct {
+}
+
+func (s int8ArraySerializer) TypeId() TypeId {
+ return INT8_ARRAY
+}
+
+func (s int8ArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s int8ArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteByte_(byte(value.Index(i).Int()))
+ }
+ return nil
+}
+
+func (s int8ArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(int8(0))) // [length]int8
Review Comment:
we can't use `[n]T` for deserialization, the `n` used by user is fixed at compile-time, but the array length can be different. We should use slice `[]numeric` for primitive array type in fory type system.
For golang serialization, we just don't allow `[n]T` being deserialized. Or only allow `[n]T` being serialized, but this seems useless. If you can't deserialzie it, serialize it seems unmeaningful too
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2760:
URL: https://github.com/apache/fory/pull/2760#discussion_r2458558045
##########
python/pyfory/tests/test_cross_language.py:
##########
@@ -338,6 +338,51 @@ def test_cross_language_serializer(data_file_path):
f.write(new_buf.get_bytes(0, new_buf.writer_index))
+@cross_language_test
+def test_cross_language_serializer_go(data_file_path):
Review Comment:
We should use same tests as java used. Otherwise, we still don't get full xlang serialization features
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "lzaeh (via GitHub)" <gi...@apache.org>.
lzaeh commented on code in PR #2760:
URL: https://github.com/apache/fory/pull/2760#discussion_r2445519047
##########
go/fory/primitive_array.go:
##########
@@ -0,0 +1,450 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package fory
+
+import (
+ "fmt"
+ "reflect"
+)
+
+type boolArraySerializer struct {
+}
+
+func (s boolArraySerializer) TypeId() TypeId {
+ return BOOL_ARRAY
+}
+
+func (s boolArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s boolArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteBool(value.Index(i).Bool())
+ }
+ return nil
+}
+
+func (s boolArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(true)) // [length]bool
+ r = reflect.New(arrT).Elem()
+
+ default:
+ return fmt.Errorf("unsupported kind %v, want slice/array/interface", type_.Kind())
+ }
+
+ for i := 0; i < length; i++ {
+ b := buf.ReadBool()
+ r.Index(i).SetBool(b)
+ }
+
+ value.Set(r)
+ return nil
+}
+
+type int8ArraySerializer struct {
+}
+
+func (s int8ArraySerializer) TypeId() TypeId {
+ return INT8_ARRAY
+}
+
+func (s int8ArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s int8ArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteByte_(byte(value.Index(i).Int()))
+ }
+ return nil
+}
+
+func (s int8ArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(int8(0))) // [length]int8
Review Comment:
Sorry for the delay—I just wrapped up my graduation stuff. If all arrays are deserialized into slices, should arrays inside structs also be deserialized as slices? If so, I think the tests will need to be updated as well.
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on code in PR #2760:
URL: https://github.com/apache/fory/pull/2760#discussion_r2458556218
##########
python/pyfory/tests/test_cross_language.py:
##########
@@ -338,6 +338,51 @@ def test_cross_language_serializer(data_file_path):
f.write(new_buf.get_bytes(0, new_buf.writer_index))
+@cross_language_test
+def test_cross_language_serializer_go(data_file_path):
Review Comment:
why need a new test for go specially?
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang closed pull request #2760: fix(go): adjustments to the sliceSerializer
URL: https://github.com/apache/fory/pull/2760
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on PR #2760:
URL: https://github.com/apache/fory/pull/2760#issuecomment-3402439950
@lzaeh could you merge main branch first? there are some conflict
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "lzaeh (via GitHub)" <gi...@apache.org>.
lzaeh commented on PR #2760:
URL: https://github.com/apache/fory/pull/2760#issuecomment-3532982012
> Could you refer to both java and rust implementation? In fory rust, both array and vec are supported, they can all be serialized as a list, and deserialize a list into target type. We should do similiar thing in go too.
>
> For fory rust, you can refer to `list.rs` and `array.rs`.
Sorry—I was tied up with some real-life errands and didn’t see this message in time.
I’ve implemented a version along the lines you suggested by removing the array serializers. Details:
(1) Primitive arrays: unless the user provides a specific destination type (only array or slice are considered valid), deserialization checks the type and length and deserializes accordingly; otherwise (e.g., when the target is interface{}) we always deserialize to a slice.
This is reflected in some local tests (still supporting array as a receiver, and when the receiver is interface{} we convert and compare the produced slice against the expected array), as well as in the cross-language test(TestXLangSerializer) where arrays are deserialized as slices.
(2) I think the support in (1) is still necessary. While we don’t recommend arrays to users, if a struct field is defined as an array (rather than interface{}), deserialization will work fine because of (1).
(3) One confusing point: the complex-struct tests had been failing for a while. For various reasons I kept pulling the latest code and reinstalling pyfory. I did the same this time, and before I had a chance to dig into the complex-struct issue, the tests started passing again.
I only opened GitHub after finishing this part of the code—bad workflow on my side T_T . I’ll follow your suggestions going forward. Thanks!
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "lzaeh (via GitHub)" <gi...@apache.org>.
lzaeh commented on code in PR #2760:
URL: https://github.com/apache/fory/pull/2760#discussion_r2445519047
##########
go/fory/primitive_array.go:
##########
@@ -0,0 +1,450 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package fory
+
+import (
+ "fmt"
+ "reflect"
+)
+
+type boolArraySerializer struct {
+}
+
+func (s boolArraySerializer) TypeId() TypeId {
+ return BOOL_ARRAY
+}
+
+func (s boolArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s boolArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteBool(value.Index(i).Bool())
+ }
+ return nil
+}
+
+func (s boolArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(true)) // [length]bool
+ r = reflect.New(arrT).Elem()
+
+ default:
+ return fmt.Errorf("unsupported kind %v, want slice/array/interface", type_.Kind())
+ }
+
+ for i := 0; i < length; i++ {
+ b := buf.ReadBool()
+ r.Index(i).SetBool(b)
+ }
+
+ value.Set(r)
+ return nil
+}
+
+type int8ArraySerializer struct {
+}
+
+func (s int8ArraySerializer) TypeId() TypeId {
+ return INT8_ARRAY
+}
+
+func (s int8ArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s int8ArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteByte_(byte(value.Index(i).Int()))
+ }
+ return nil
+}
+
+func (s int8ArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(int8(0))) // [length]int8
Review Comment:
Sorry for the delay—I just wrapped up my graduation stuff. If all arrays are deserialized into slices, should arrays inside structs also be deserialized as slices? If so, I think the tests will need to be updated as well.
##########
go/fory/primitive_array.go:
##########
@@ -0,0 +1,450 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package fory
+
+import (
+ "fmt"
+ "reflect"
+)
+
+type boolArraySerializer struct {
+}
+
+func (s boolArraySerializer) TypeId() TypeId {
+ return BOOL_ARRAY
+}
+
+func (s boolArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s boolArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteBool(value.Index(i).Bool())
+ }
+ return nil
+}
+
+func (s boolArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(true)) // [length]bool
+ r = reflect.New(arrT).Elem()
+
+ default:
+ return fmt.Errorf("unsupported kind %v, want slice/array/interface", type_.Kind())
+ }
+
+ for i := 0; i < length; i++ {
+ b := buf.ReadBool()
+ r.Index(i).SetBool(b)
+ }
+
+ value.Set(r)
+ return nil
+}
+
+type int8ArraySerializer struct {
+}
+
+func (s int8ArraySerializer) TypeId() TypeId {
+ return INT8_ARRAY
+}
+
+func (s int8ArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s int8ArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteByte_(byte(value.Index(i).Int()))
+ }
+ return nil
+}
+
+func (s int8ArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(int8(0))) // [length]int8
Review Comment:
> we can't use `[n]T` for deserialization, the `n` used by user is fixed at compile-time, but the array length can be different. We should use slice `[]numeric` for primitive array type in fory type system.
>
> For golang serialization, we just don't allow `[n]T` being deserialized. Or only allow `[n]T` being serialized, but this seems useless. If you can't deserialzie it, serialize it seems unmeaningful too
Sorry for the delay—I just wrapped up my graduation stuff. If all arrays are deserialized into slices, should arrays inside structs also be deserialized as slices? If so, I think the tests will need to be updated as well.
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "chaokunyang (via GitHub)" <gi...@apache.org>.
chaokunyang commented on PR #2760:
URL: https://github.com/apache/fory/pull/2760#issuecomment-3517541275
Could you refer to both java and rust implementation? In fory rust, both array and vec are supported, they can all be serialized as a list, and deserialize a list into target type.
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "lzaeh (via GitHub)" <gi...@apache.org>.
lzaeh commented on code in PR #2760:
URL: https://github.com/apache/fory/pull/2760#discussion_r2445520383
##########
go/fory/primitive_array.go:
##########
@@ -0,0 +1,450 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package fory
+
+import (
+ "fmt"
+ "reflect"
+)
+
+type boolArraySerializer struct {
+}
+
+func (s boolArraySerializer) TypeId() TypeId {
+ return BOOL_ARRAY
+}
+
+func (s boolArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s boolArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteBool(value.Index(i).Bool())
+ }
+ return nil
+}
+
+func (s boolArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Bool {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(true)) // [length]bool
+ r = reflect.New(arrT).Elem()
+
+ default:
+ return fmt.Errorf("unsupported kind %v, want slice/array/interface", type_.Kind())
+ }
+
+ for i := 0; i < length; i++ {
+ b := buf.ReadBool()
+ r.Index(i).SetBool(b)
+ }
+
+ value.Set(r)
+ return nil
+}
+
+type int8ArraySerializer struct {
+}
+
+func (s int8ArraySerializer) TypeId() TypeId {
+ return INT8_ARRAY
+}
+
+func (s int8ArraySerializer) NeedWriteRef() bool {
+ return true
+}
+
+func (s int8ArraySerializer) Write(f *Fory, buf *ByteBuffer, value reflect.Value) error {
+ if value.Kind() == reflect.Interface {
+ value = value.Elem()
+ }
+ size := value.Len()
+ if size >= MaxInt32 {
+ return fmt.Errorf("too long slice: %d", size)
+ }
+ buf.WriteLength(size)
+ for i := 0; i < size; i++ {
+ buf.WriteByte_(byte(value.Index(i).Int()))
+ }
+ return nil
+}
+
+func (s int8ArraySerializer) Read(f *Fory, buf *ByteBuffer, type_ reflect.Type, value reflect.Value) error {
+ length := buf.ReadLength()
+ var r reflect.Value
+ switch type_.Kind() {
+ case reflect.Slice:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ r = reflect.MakeSlice(type_, length, length)
+
+ case reflect.Array:
+ if type_.Elem().Kind() != reflect.Int8 {
+ return fmt.Errorf("element kind must be bool, got %v", type_.Elem())
+ }
+ if length != type_.Len() {
+ return fmt.Errorf("length %d does not match array type %v", length, type_)
+ }
+ r = reflect.New(type_).Elem()
+
+ case reflect.Interface:
+ arrT := reflect.ArrayOf(length, reflect.TypeOf(int8(0))) // [length]int8
Review Comment:
> we can't use `[n]T` for deserialization, the `n` used by user is fixed at compile-time, but the array length can be different. We should use slice `[]numeric` for primitive array type in fory type system.
>
> For golang serialization, we just don't allow `[n]T` being deserialized. Or only allow `[n]T` being serialized, but this seems useless. If you can't deserialzie it, serialize it seems unmeaningful too
Sorry for the delay—I just wrapped up my graduation stuff. If all arrays are deserialized into slices, should arrays inside structs also be deserialized as slices? If so, I think the tests will need to be updated as well. @chaokunyang
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "lzaeh (via GitHub)" <gi...@apache.org>.
lzaeh closed pull request #2760: fix(go): adjustments to the sliceSerializer
URL: https://github.com/apache/fory/pull/2760
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "pandalee99 (via GitHub)" <gi...@apache.org>.
pandalee99 commented on PR #2760:
URL: https://github.com/apache/fory/pull/2760#issuecomment-3438125388
>Update the struct field ordering logic accordingly. After updating the Python module, two struct tests failed due to hash mismatches. I haven’t changed the handling of complex structs, which means the code path for fields that are arrays within complex structs remains untouched—and this test includes arrays. If I tweak just one spot now, it’ll be hard to localize the issue. It’s better to make the corresponding changes after I spend time learning the new sorting/ordering optimization algorithm, so we can update everything in sync.
Recently, we have updated the hash calculation algorithm. Maybe this PR(#2814) can solve this problem
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org
Re: [PR] fix(go): adjustments to the sliceSerializer [fory]
Posted by "lzaeh (via GitHub)" <gi...@apache.org>.
lzaeh commented on PR #2760:
URL: https://github.com/apache/fory/pull/2760#issuecomment-3532996998
I’ll keep studying those two files and figure out suitable approach for Go. This version is only based on earlier discussion and hasn’t yet incorporated learnings from the other language implementations. :(
--
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: commits-unsubscribe@fory.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fory.apache.org
For additional commands, e-mail: commits-help@fory.apache.org