Pass target arrow type to array_decoder_factory

[progval]

This is the prerequisite to support configurable timestamp precision mentioned in #75 (comment)

[Jefffrey]

I plan to take a look at this later today 👍

[waynexia]

Thanks @progval, this API looks good to me overall 👍

One thing I'm concerning is we may also need to provide a method that can convert ORC type to Arrow type, the so-called "default" option (or there is one and I miss it?). Otherwise the external user of array_decoder_factory needs to maintain that mapping by themselves.

[waynexia]

The key API change 👍

[progval]

One thing I'm concerning is we may also need to provide a method that can convert ORC type to Arrow type, the so-called "default" option (or there is one and I miss it?). Otherwise the external user of array_decoder_factory needs to maintain that mapping by themselves.

Yes, I'm planning to add it after #93 is merged. Probably .with_default_time_unit(TimeUnit) or .with_default_timestamp_type::<T: arrow::datatypes::ArrowTimestampType>() on the reader builder

[Jefffrey]

Do you think it's possible to store the Field inside Column itself, considering it's already storing DataType?

datafusion-orc/src/column.rs

Lines 15 to 21 in ac5a8ab

	#[derive(Debug)]
	pub struct Column {
	number_of_rows: u64,
	footer: Arc<StripeFooter>,
	name: String,
	data_type: DataType,
	}

Might cut down on having to pass field everywhere 🤔

[progval]

Do you think it's possible to store the Field inside Column itself, considering it's already storing DataType?

datafusion-orc/src/column.rs

Lines 15 to 21 in ac5a8ab

	#[derive(Debug)]
	pub struct Column {
	number_of_rows: u64,
	footer: Arc<StripeFooter>,
	name: String,
	data_type: DataType,
	}

Might cut down on having to pass field everywhere 🤔

It looked like a good idea, but it actually complexifies everything:

1. matching both types was only in array_decoder/mod.rs while destructuring the ORC type, but is now spread between that module, column.rs, and array_decoder/{struct,list,map,union}.rs. In particular, the union decoder expects UnionFields which is an iterable of (i8, Field) unlike Fields that every other one would use, so it can't rely on Column::children() to match both types
2. Column::children() now needs to return a Result
3. We would need the Field when constructing the Column, so we need to pass Fields through Stripe::new, so also in StripeFactory
4. Overall it makes the code larger and less readable. See my attempt: progval/datafusion-orc@target-type...progval:datafusion-orc:target-type-in-column (which doesn't even include all of point 3, or moving validation that both types match from array_decoder_factory to Column::new to make sure the Column is self-consistent)

[Jefffrey]

Thanks for this, and thanks for the detailed breakdown on my suggestion ❤️