Re: [PR] Fix(avro-reader): bunch of types that didn't work [arrow-go]
Willem-J-an commented on PR #416: URL: https://github.com/apache/arrow-go/pull/416#issuecomment-3047694790 Added the schema file to the rat exclude, that should fix the windows tests. Any other feedback I'd need to look at? 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Fix(avro-reader): bunch of types that didn't work [arrow-go]
Willem-J-an commented on PR #416: URL: https://github.com/apache/arrow-go/pull/416#issuecomment-3046221636 I added working fixes for all the types already, including for fixed! -- 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]
Re: [PR] Fix(avro-reader): bunch of types that didn't work [arrow-go]
zeroshade commented on PR #416: URL: https://github.com/apache/arrow-go/pull/416#issuecomment-3046168352 A `fixed` should probably map to a `fixed_len_byte_array[N]` if we can easily detect it, if we can't then I think it's okay to remove it for now if it's not currently working. -- 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]
Re: [PR] Fix(avro-reader): bunch of types that didn't work [arrow-go]
Willem-J-an commented on PR #416: URL: https://github.com/apache/arrow-go/pull/416#issuecomment-3045997065  My example was fixed binary, notice it shows up twice in above screenshot. I observed the fixed binary to return an array, as is described in the screenshot as well. The case didn't apply because when I use arrow reader it converts to array, not to map and not to slice. I don't know if there's a scenario where it would actually be used, that's my question! I suppose if hamba doesn't decode into slice or map for fixed bytes, is there any other decoder that users can use that do hit these code paths? If not than maybe these are only applicable to hamba v1 or so, and not needed anymore. If you're unsure we can also just leave it. The most important part for me is to have the working cases added, rather than the not working cases removed. And fixed binary was an example, the same applies to timestamps and the other types I fixed. -- 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]
Re: [PR] Fix(avro-reader): bunch of types that didn't work [arrow-go]
zeroshade commented on PR #416: URL: https://github.com/apache/arrow-go/pull/416#issuecomment-3045872523 ># <- this case does not apply at least in my testing because hamba decoder returns array, not a slice That's odd and is likely a bug in hamba then, according to the docs it should return `[]bytes` correctly:  > # <- this case also didn't apply in my testing Didn't apply because we aren't testing it? or a different case gets matched for the test that *should* hit it? -- 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]
Re: [PR] Fix(avro-reader): bunch of types that didn't work [arrow-go]
zeroshade commented on PR #416: URL: https://github.com/apache/arrow-go/pull/416#issuecomment-3045867668 You can add the file to the `dev/release/rat_exclude_files.txt` that should exclude it from the license check -- 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]
Re: [PR] Fix(avro-reader): bunch of types that didn't work [arrow-go]
Willem-J-an commented on PR #416: URL: https://github.com/apache/arrow-go/pull/416#issuecomment-3045842043 Seems like my changes do not work on windows. I added the license info to the avro schema which seems a bit weird. Is there a better way to deal with the license for this file? -- 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]
Re: [PR] Fix(avro-reader): bunch of types that didn't work [arrow-go]
Willem-J-an commented on PR #416:
URL: https://github.com/apache/arrow-go/pull/416#issuecomment-3045815451
Tests that are being removed? I think no tests are being removed?
As an example, the cases that I'm referring to is this:
```
func appendFixedSizeBinaryData(b *array.FixedSizeBinaryBuilder, data
interface{}) {
switch dt := data.(type) {
case nil: # <- is fine of course
b.AppendNull()
case []byte: # <- this case does not apply at least in my testing
because hamba decoder returns array, not a slice
b.Append(dt)
case map[string]any: # <- this case also didn't apply in my testing
switch v := dt["bytes"].(type) {
case nil:
b.AppendNull()
case []byte:
b.Append(v)
}
```
Not sure if these are legacy or still relevant in cases that I cannot
oversee!
--
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]
Re: [PR] Fix(avro-reader): bunch of types that didn't work [arrow-go]
zeroshade commented on PR #416: URL: https://github.com/apache/arrow-go/pull/416#issuecomment-3045563808 Which cases do you see are no longer applicable and not working? the tests that are being removed seem to be types that should be supported -- 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]
