GH-43443: [Go] [IPC] Infer schema from first record if not specified#43484
GH-43443: [Go] [IPC] Infer schema from first record if not specified#43484joellubi merged 1 commit intoapache:mainfrom
Conversation
|
|
zeroshade
left a comment
There was a problem hiding this comment.
LGTM just one nit that may not be applicable
| fs := flightStreamWriter{} | ||
| w := flight.NewRecordWriter(&fs, ipc.WithSchema(schema)) | ||
|
|
||
| require.ErrorContains(t, w.Write(recs[0]), "arrow/ipc: tried to write record batch with different schema") |
There was a problem hiding this comment.
do we use a specific error type via %w in this message? If so, can we try using ErrorIs?
There was a problem hiding this comment.
We don't unfortunately. The definition is currently just an errString.
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit e9f6667. There were 11 benchmark results indicating a performance regression:
The full Conbench report has more details. It also includes information about 5 possible false positives for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
Fixes: #43443
Makes usage of the IPC writer and any writers that use it such the flight writer simpler.
What changes are included in this PR?
Are these changes tested?
Yes
Are there any user-facing changes?
Any
ipc.Writerthat does not specify the optional argumentipc.WithSchemawill no longer return an error as long as the incoming stream of records has a consistent schema.