zeroshade commented on code in PR #929:
URL: https://github.com/apache/arrow-go/pull/929#discussion_r3566723346
##########
arrow/array/util.go:
##########
@@ -219,14 +237,8 @@ func RecordFromJSON(mem memory.Allocator, schema
*arrow.Schema, r io.Reader, opt
o(&cfg)
}
- if cfg.startOffset != 0 {
- seeker, ok := r.(io.ReadSeeker)
- if !ok {
- return nil, 0, errors.New("using StartOffset option
requires reader to be a ReadSeeker, cannot seek")
- }
- if _, err := seeker.Seek(cfg.startOffset, io.SeekStart); err !=
nil {
- return nil, 0, fmt.Errorf("failed to seek to start
offset %d: %w", cfg.startOffset, err)
- }
+ if err := seekJSONStartOffset(r, cfg.startOffset); err != nil {
Review Comment:
Since this shared helper now backs `RecordFromJSON` too, please add a direct
`RecordFromJSON` test (one seek-failure path + one successful-offset path); the
new tests currently only exercise `FromJSON`.
##########
arrow/array/util.go:
##########
@@ -74,6 +74,29 @@ func WithUseNumber() FromJSONOption {
}
}
+func seekJSONStartOffset(r io.Reader, offset int64) error {
Review Comment:
Note: the error text changed (e.g. `RecordFromJSON` previously returned
`failed to seek to start offset %d`). If any downstream code/tests assert on
the old message, they'll need updating.
--
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]
