The flight.NewRecordWriter parameter is ambiguous

[mac-zhenfang]

Describe the usage question you have. Please include as many useful details as possible.

The func NewRecordWriter(w DataStreamWriter, opts ...ipc.Option) *Writer API indicates the DataStreamWriter is a required parameter, all the others are optional. But in the

func (w *Writer) start() error {
w.started = true

w.mapper.ImportSchema(w.schema)
w.lastWrittenDicts = make(map[int64]arrow.Array)

// write out schema payloads
ps := payloadFromSchema(w.schema, w.mem, &w.mapper)
defer ps.Release()

for _, data := range ps {
	err := w.pw.WritePayload(data)
	if err != nil {
		return err
	}
}

return nil

}

The w.schema looks a required parameter. If it s nil, will report arrow/ipc: unknown error while writing: runtime error: invalid memory address or nil pointer dereference error.

The request is to use the Schema in RecordBatch, instead of a input option

Component(s)

Go

[joellubi]

Thanks for opening this. It indeed is the case that the schema is logically a required parameter; the IPC stream must have a consistent schema.

I agree that having it as an optional parameter may be misleading. The reason for this is because it passes through the general ipc.Option's that are shared across the different IPC readers/writers. Often the schema is optional because it can be inferred otherwise, such as from the footer of an IPC file in the case of reading. In that specific case the schema is not required, but if it is provided a check is performed to ensure it matches the footer schema.

I think a similar approach could make sense here in the case of the flight.Writer:

• If no schema is provided in the constructor, set it for the whole stream based off the first record written. All subsequent records must match this schema.
• If a schema is provided, check the first record's schema matches and proceed as before.

@zeroshade Do these semantics seem reasonable?

[zeroshade]

@joellubi Yup, those are definitely reasonable. and a good idea for us to make the change.

[joellubi]

Issue resolved by pull request 43484
#43484