Unable to use time::OffsetDateTime (and likely other time types) in binds

[v3xro]

Binding a query like client.query("SELECT ?fields WHERE ts > ?").bind(datetime!(2022-11-13 15:27:42 UTC)) results in runtime execution error:

DB::Exception: Illegal type Tuple(UInt16, UInt8, UInt8, UInt8, UInt8, UInt32, UInt8, UInt8, UInt8)

It seems the issue is there isn't a specific serializer for these time types and ClickHouse does not accept the default representation serde generates.

[v3xro]

Workaround is to implement your own serialization for a newtype struct around OffsetDateTime, something like:

use serde::{ser::Error, Deserialize, Serialize, Serializer};
use time::{
    format_description::well_known::iso8601::{Config, EncodedConfig, FormattedComponents},
    OffsetDateTime,
};

struct SafeDateTime(OffsetDateTime);

const CLICKHOUSE_CONFIG: EncodedConfig = Config::DEFAULT
    .set_formatted_components(FormattedComponents::DateTime)
    .encode();


impl Serialize for SafeDateTime {
      fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error> where S: Serializer {
    self.0.format(&Iso8601::<CLICKHOUSE_CONFIG>)
            .map_err(S::Error::custom)?
            .serialize(serializer)
}
}

[loyd]

Yes =( It's exactly what I do:

.bind(interval.start_time.as_rfc3339_weak())

I don't think it's possible without specialization in Rust (what won't happen soon) to have a dedicated logic.

Another way is to provide wrappers by the crate.

[v3xro]

You could remove the blanket S: Serialize implementation for Bind though and implement the types that can be bound more precisely (which would allow the time types to live under the time feature) and prevent default serialization behavior (or have it a second variant - probably the serialize option is more performant when you're actually inserting data)

[loyd]

You could remove the blanket S: Serialize implementation

It's not obvious, because it's very useful, and my production code relies heavily on it: we use a lot of wrappers like Timestamp, custom Duration, and so on working because of this blanket implementation. Yep, they can implement the Bind trait directly, but it requires stabilization of this trait and leads to strange extra dependencies between crates: all crates that provide custom types should rely on the clickhouse crate only because of the Bind trait.

It should be much easier after stabilizing specialization in rustc

[v3xro]

Even min_specialization looks like it will make take more time to land outside of nightly though. As you said above, exposing Bind isn't ideal either.

Another idea could be to have a feature flag auto_serialize or no_auto_serialize probably that drops this impl <S: Serialize> Bind for S behavior unless S: Serialize + Auto so you would have to opt-in to serialization for other types. Of course this won't work unless you also create a conversion into this Auto for any type for coherence rules but I feel like this could work if you wanted to have strictly more safety than a blanket serialize impl. Still not ideal though.