optimize values list execution plan by moving the evaluation part to execution phase

[jimexist]

I wonder what you think about moving the creation of the actual RecordBatch from ValuesExec::try_new to execute -- the rationale would be to make PhysicalPlan creation faster and push the actual work into execute where if can potentially be run concurrently with other parts

Given the size of data in a VALUES statement, this is not likely to be any real difference so I am fine with leaving the creation in the same place too -- I just wanted to mention it.

Originally posted by @alamb in #1165 (comment)

[ygf11]

@alamb @jimexist It seems not hard, I want have a try!

[alamb]

Thanks @ygf11 !

[ygf11]

It seems statistics computation in ValuesExec also needs the evaluation result.
We need a RwLock?
https://github.com/apache/arrow-datafusion/blob/6b964cbfedc3ee5b08dea8a6c598de7844b95e91/datafusion/src/physical_plan/values.rs#L164-L168

[alamb]

I think don't think adding an RwLock sounds like a good idea. Perhaps we could compute the statistics when the ValuesExec is created (rather than computing on the call to statistics(&self)?

[jimexist]

I think don't think adding an RwLock sounds like a good idea. Perhaps we could compute the statistics when the ValuesExec is created (rather than computing on the call to statistics(&self)?

agreed, no need for a lock. we can just put evaluation in the creation part of ValuesExec. i mean it probably wouldn't make any difference

[ygf11]

@alamb IMOP, earlier computing statistics conflicts with the purpose of this task, because this will iterate all row and column, which also slow the creation of ValueExec. Maybe stay the same is ok?

I think don't think adding an RwLock sounds like a good idea. Perhaps we could compute the statistics when the ValuesExec is created (rather than computing on the call to statistics(&self)?

[alamb]

@ygf11 that is an excellent point -- perhaps then we should just close this task as "won't do" ?