Partial impl<'a> From<&'a Value> for ValueRef<'a> causes panics

[ianthetechie]

Title says it all. I got a bit of a nasty surprise when trying to pass a list of duckdb::types::value::Value::List as params when executing a statement. I'm sure there is some other way of passing a list of strings as a parameter, but I was quite surprised when I got a runtime panic due to the partial implementation of From. This should probably be TryFrom if it's not going to be implemented.

duckdb-rs/crates/duckdb/src/types/value_ref.rs

Lines 352 to 355 in 19acb89

	Value::Enum(..) => todo!(),
	Value::List(..) | Value::Struct(..) | Value::Map(..) | Value::Array(..) | Value::Union(..) => {
	unimplemented!()
	}

(Happy to work on a solution by the way; I don't quite grok what's supposed to be happening in this impl, but with a few pointers I'd be happy to work up a PR.

[rm-dr]

I agree. This is a bit annoying, as it makes code that theoretically works panic.

There isn't much we can do about this, though, until we implement the missing types!

[ianthetechie]

Maybe I missed something but does this have to be a From impl? I think that partial or failable conversions are the reason TryFrom exists.

[rm-dr]

This isn't a fallible conversion---it's simply unimplemented!
TryFrom might make sense, if the conversion is fallible when implemented.

[molysgaard]

I'm hitting this issue right now.
I want to insert some rows, where one of the columns is an Array<f64> (length of array is 500 floats). The only way I am able to do it is by converting the whole float array into a big fat string, and then parsing that using the ToSqlOutput::from(big_fat_string). This is extremely slow, and even with a super-optimized implementation I am only able to squeeze out 6000 rows/sec in a tight append_row loop.

I have done an identical setup in Postgres, and there i am able to get 20000 rows/sec from the same rust application. DuckDb being slower than Postgres, at duckdb's key value proposition (bulk data loading), that is not good!

I do have some cycles to implement this, but looking at the code, it seems like there is some fundamental issues with the ValueRef datatype. It is not really a reference to underlying data, it copies everything. That is why it is not implemented for non-primitive datatypes?
I also see the appender implementation does not handle non-primitive datatypes at all. This seems half cooked?

[rm-dr]

Yeah, it's a bit odd. I don't think it's possible to implement these traits for complex types. Arrow data may need some kind conversion, which requires allocation, but we cannot return a ref to data allocated locally in a fn.

[mlafeldt]

Lack of support for container types is tracked in #680. Keeping this issue open as From would require a separate fix.