serramatutu commented on PR #833: URL: https://github.com/apache/arrow-go/pull/833#issuecomment-5601857725
@zeroshade regarding the previous comments: I have fixed the perf issue, now the row buffering only happens at the top-level struct, and we avoid re-decoding nested rows. Had to add some plumbing to `Builder` for that. About the other comments: > arrow/array/builder.go:451 unconditionally enables UseNumber() while replaying buffered rows, overriding the caller's decoder semantics. I kept it like this as I assume this is intender behavior for 2 reasons: - `WithUseNumber()` is deprecated and the docs state it's a no-op and always enabled by default. So I'm always enabling it. - Not enabling it makes the JSON decoder lenient as it can decode floats like `1.5` into integers by truncating you, silently violating the schema. There was a test that asserted that, which I have fixed (see my other comment above on that test) > arrow/array/builder.go:515-521 skips unknown fields before recording seen keys, so duplicate unknown keys are silently accepted. Also intended behavior. I added a comment that the decoder behaves like the default C++ decoder, with [`ParseOptions::Ignore`])https://github.com/apache/arrow/blob/05c1a4bf545a560f2b118a2e6e6d3d6fa36a2716/cpp/src/arrow/json/options.h#L35(, which ignores unknown keys. I added it as a TODO comment to possibly support `ParseOptions` with `Error` and `InferType` as well for strict/lenient parsing. -- 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]
