feat(rust)!: return Box<RecordBatchReader + 'static> for caller flexibility

[lidavidm]

This avoids inadvertently tying the result lifetime to the lifetimes of any input arguments. We discussed more drastic changes like changing the result to have 'static or even using Box<Reader + 'static>, but this opts for the most conservative change.

Closes #2694.

[lidavidm]

CC @eitsupi as well.

That said, I'm still thinking about whether it should be Box<RecordBatchReader + 'static>. This would definitely constrain driver implementations much more but would be easiest to use for consumers (albeit with some more possibility of dynamic runtime errors, but maybe that's actually what we want?)

[felipecrv]

This is a good initial step. It's the first change I added to my fork. But real-world usage really requires a Box<dyn RecordBatchReader>.

The impl RecordBatchReader only covers cases where you get the reader and fully consume it right after the call.

[paleolimbot]

I'm not an expert on Rust lifetimes but this does seem like an improvement. My specific use case is writing a driver that doesn't have lifetime constraints on its readers (DataFusion) where I'm immediately exporting to FFI where the lifetime constraints are no longer enforced, so I'm probably a poor judge on both the use and author side of this.

[lidavidm]

Ok. In that case I'll update this PR to go all the way. I figure it's better to have only one breaking change.

[paleolimbot]

Thank you!

[lidavidm]

Rebased; I'll merge if there's no objections

[felipecrv]

I'm not an expert on Rust lifetimes but this does seem like an improvement. My specific use case is writing a driver that doesn't have lifetime constraints on its readers (DataFusion) where I'm immediately exporting to FFI where the lifetime constraints are no longer enforced, so I'm probably a poor judge on both the use and author side of this.

Your intuition is right! Returning an impl tied to the Statement object lifetime is too limiting. It only covers unit test cases where you create a statement, execute it and fully consume the reader without storing it anywhere else other than the stack.

[felipecrv]

Thank you @lidavidm. This is great.