zeroshade commented on code in PR #1139:
URL: https://github.com/apache/arrow-go/pull/1139#discussion_r3769759719
##########
arrow/ipc/file_writer.go:
##########
@@ -285,6 +286,12 @@ func NewFileWriter(w io.Writer, opts ...Option)
(*FileWriter, error) {
}
func (f *FileWriter) Close() error {
+ if f.closed {
+ return nil
+ }
+ f.closed = true
Review Comment:
Setting `closed` before the close operation succeeds causes a failed close
to become success on every subsequent call. For example, when the output sink
rejects the initial header:
```go
require.ErrorIs(t, writer.Close(), sinkErr)
require.ErrorIs(t, writer.Close(), sinkErr) // currently returns nil
```
This probe passes on the previously reviewed head and fails after this
change. Since the writer is terminal after the first call, please retain and
return the close error on subsequent calls, consistent with the stream
`Writer`, rather than returning `nil`. Please add regression coverage for
repeated `Close` after a sink failure.
--
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]
