Change return type of 'DataFrame.collect()'

[kination]

Which issue does this PR close?

It is for following issue #47

What changes are included in this PR?

Change return type of 'DataFrame.collect()' to 'SendableRecordBatchStream'.

Currently this is draft change, so please let me know if this change is not the one you've intended 🙏

[alamb]

This PR appears to change the signature of the free function datafusion::physical_plan::collect -- I think the definition of DataFrame::collect is here:
https://github.com/apache/arrow-datafusion/blob/master/datafusion/src/dataframe.rs#L221

@andygrove is this what you had in mind in #47?

[andygrove]

The change in this PR looks like a good start and probably necessary, but yes, the goal is to update DataFrame::collect

[kination]

@alamb @andygrove thanks for review. Will fix soon 🙏

As far as I review from now, seems it needs to fix return format of collect inside dataframe_impl. And I've fixed collect inside physical_plan because they are calling this function inside...

Please let me know if there's any misunderstanding 🙇

[alamb]

@djKooks do you plan to keep working on this PR?

[kination]

@alamb sure~
But because I'm still on reviewing the project, so want to make sure I'm going in right way...

As far as I review from now, seems it needs to fix return format of collect inside dataframe_impl. And I've fixed collect inside physical_plan because they are calling this function inside...
Please let me know if there's any misunderstanding 🙇

=> Am I thinking correctly?

[alamb]

Thank you @djKooks

#47
I think is referring to DataFrame::collect(), https://docs.rs/datafusion/4.0.0/datafusion/dataframe/trait.DataFrame.html#tymethod.collect not physical_plan::collect() which is what this PR currently does

@andygrove can you please weigh in here on the desired API?

#47 says collect() , however, collect() elsewhere in the code returns Vec<RecordBatch>

Perhaps would it make sense to add a new function like DataFrame::execute() that returned a SendableRecordBatch stream and leave collect() the way it is?

`

[andygrove]

Sorry for the delayed response. I have been working on related areas and plan to look at this later today or tomorrow.

[andygrove]

I have created #789 to implement streaming versions of the collect methods.

[kination]

@andygrove okay. Will close this

[alamb]

Thanks for prodding this along @djKooks