Use DataFusionError instead of ArrowError in FileOpenFuture

[adriangb]

I would like to re-use ProjectionStream for #14993:

datafusion/datafusion/physical-plan/src/projection.rs

Lines 362 to 388 in 0a9dcec

	struct ProjectionStream {
	schema: SchemaRef,
	expr: Vec<Arc<dyn PhysicalExpr>>,
	input: SendableRecordBatchStream,
	baseline_metrics: BaselineMetrics,
	}
	impl Stream for ProjectionStream {
	type Item = Result<RecordBatch>;
	fn poll_next(
	mut self: Pin<&mut Self>,
	cx: &mut Context<'_>,
	) -> Poll<Option<Self::Item>> {
	let poll = self.input.poll_next_unpin(cx).map(|x| match x {
	Some(Ok(batch)) => Some(self.batch_project(&batch)),
	other => other,
	});
	self.baseline_metrics.record_poll(poll)
	}
	fn size_hint(&self) -> (usize, Option<usize>) {
	// Same number of record batches
	self.input.size_hint()
	}
	}

A barrier to this is that the FileOpenerFuture stream uses ArrowError while the ExecutionPlan::execute stream uses DataFusionError. I don't see any reason for this so I propose we change it.

Within DataFusion we should not really be creating ArrowErrors, I believe we should almost always use DataFusionError.

Upgrade guide note aside this is a -3 LOC diff -> reducing complexity (less error types, less lines of code).

[comphead]

Suggested change

	datafusion_common::DataFusionError::Execution(format!(
	exec_err!(

[comphead]

Suggested change

	let error = futures::future::ready(Err(DataFusionError::Execution(
	let error = futures::future::ready(exec_err!(

[comphead]

Result<RecordBatch, DataFusionError> can be replaced with pub type Result<T, E = DataFusionError> = result::Result<T, E>; declared in datafusion/common/src/error.rs

[adriangb]

Thanks @comphead ! Addressed your feedback

[comphead]

This is amazing how rustc can distinguish Result implementations 😮

[comphead]

should the Item also reuse DF Result?

[comphead]

lgtm thanks @adriangb

[adriangb]

I also spoke with @alamb about this change, he agreed it's a positive change and that the breakage should be minimal because (1) not many people are implementing FileOpenFuture and (2) there is an impl From<ArrowError> for DataFusionError.

[alamb]

Also, I think the File opener trait is somewhat specialized / not used all that widely, so this change will likely not affect that many people