suharev7 / clickhouse-rs

Asynchronous ClickHouse client library for Rust programming language.
MIT License
324 stars 121 forks source link

Inserting DateTime64 through row! macro causes a crash #134

Open prk3 opened 3 years ago

prk3 commented 3 years ago

Hi! I have trouble inserting DateTime64 values into a table that looks like this:

CREATE TABLE default (
    `id` UInt32,
    `date` DateTime64(9, 'UTC')
)
ENGINE = MergeTree()
PRIMARY KEY id

The following code produces an error (runtime error):

let date = chrono_tz::UTC.ymd(2020, 2, 3).and_hms_nano(13, 45, 50, 8927265);

let mut b = clickhouse_rs::types::Block::new();

b.push(clickhouse_rs::row!{ // notice the use of row macro for adding elements to a block
    id: 1u32,
    date,
}).unwrap();

dbg!(b.get_column("date").unwrap().sql_type()); // prints DateTime(DateTime32)

client.insert("default", b)

Here is the error:

Err(
    FromSql(
        InvalidType {
            src: "DateTime",
            dst: "DateTime64(9, \'UTC\')",
        },
    ),
)

As far as I understand, it says that clickhouse expected DateTime64 and received DateTime. I tried indicating that I want DateTime64 with clickhouse_rs::Value::DateTime64:

b.push(clickhouse_rs::row!{ // notice the use of row macro for adding elements to a block
    id: 1u32,
    date: clickhouse_rs::types::Value::DateTime64(date.timestamp_nanos(), (9, chrono_tz::UTC)),
}).unwrap();

dbg!(b.get_column("date").unwrap().sql_type()); // prints DateTime(DateTime64(9, UTC))

client.insert("default", b)

But that causes a panic:

thread 'tokio-runtime-worker-0' panicked at 'not implemented', clickhouse-rs/src/types/column/datetime64.rs:53:9
stack backtrace:
   0: rust_begin_unwind
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/std/src/panicking.rs:493:5
   1: core::panicking::panic_fmt
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/panicking.rs:92:14
   2: core::panicking::panic
             at /rustc/2fd73fabe469357a12c2c974c140f67e7cdd76d0/library/core/src/panicking.rs:50:5
   3: <clickhouse_rs::types::column::datetime64::DateTime64ColumnData as clickhouse_rs::types::column::column_data::ColumnData>::save
             at ./clickhouse-rs/src/types/column/datetime64.rs:53:9
   4: <clickhouse_rs::types::column::chunk::ChunkColumnData as clickhouse_rs::types::column::column_data::ColumnData>::save
             at ./clickhouse-rs/src/types/column/chunk.rs:30:9
   5: clickhouse_rs::types::column::Column<K>::write
             at ./clickhouse-rs/src/types/column/mod.rs:217:9
   6: clickhouse_rs::types::block::Block::write
             at ./clickhouse-rs/src/types/block/mod.rs:316:17
   7: clickhouse_rs::types::block::Block::send_data
             at ./clickhouse-rs/src/types/block/mod.rs:325:13
   8: clickhouse_rs::types::cmd::encode_data
             at ./clickhouse-rs/src/types/cmd.rs:138:5
   9: clickhouse_rs::types::cmd::encode_command
             at ./clickhouse-rs/src/types/cmd.rs:36:42
  10: clickhouse_rs::types::cmd::encode_union
             at ./clickhouse-rs/src/types/cmd.rs:143:22
  11: clickhouse_rs::types::cmd::encode_command
             at ./clickhouse-rs/src/types/cmd.rs:37:38
  12: clickhouse_rs::types::cmd::Cmd::get_packed_command
             at ./clickhouse-rs/src/types/cmd.rs:21:9
  13: clickhouse_rs::io::transport::ClickhouseTransport::send
             at ./clickhouse-rs/src/io/transport.rs:200:37
  14: <clickhouse_rs::io::transport::PacketStream as futures::stream::Stream>::poll
             at ./clickhouse-rs/src/io/transport.rs:293:36

I found a workaround: inserting columns, not rows.

let b = clickhouse_rs::types::Block::new()
   .add_column("id", vec![1u32])
   .add_column("date", vec![date]);

dbg!(b.get_column("date").unwrap().sql_type()); // prints DateTime(Chrono)

client.insert("default", b)

Is this a bug or am I using raw! macro wrong? What's the intended way of passing DateTime64 to row?

suharev7 commented 3 years ago

Hi! It's a bug in clickhouse-rs. Fix has already been pushed in this repository, and it will be in the next release.

rsalmei commented 2 years ago

Hey, has this fix been released?

rsalmei commented 2 years ago

I see it isn't, I've tried the same workaround as @prk3, and got the exact same panic... 😞 I also tried to use the latest source here, directly from github's async-await branch, which is the default, but it is also breaks with error: failed to select a version for 'pin-project'. Can you please help?

KuangyeChen commented 2 years ago

Still got this error.

rsalmei commented 2 years ago

😞

suharev7 commented 2 years ago

It's example witch show that code above work without any errors. Could you please give me an example that reproduces this error?

rsalmei commented 2 years ago

@prk3 explained it perfectly in the original post, the error is with DateTime64, not the 32 your example uses. Thanks.

suharev7 commented 2 years ago

Actual, no. It creates a column of Chrono which will be cast to DateTime64.

dbg!(b.get_column("date").unwrap().sql_type()); // prints DateTime(Chrono)
rsalmei commented 2 years ago

Not with the released version on crates "1.0.0-alpha.1". I get this:

[src/tasks/clickhouse_pusher.rs:52] b.get_column("date").unwrap().sql_type() = DateTime(
    DateTime32,
)
Error in clickhouse pusher: From SQL error: `SqlType::DateTime cannot be cast to DateTime64(9, 'UTC').`

Caused by:
    SqlType::DateTime cannot be cast to DateTime64(9, 'UTC').
rsalmei commented 2 years ago

Fix has already been pushed in this repository, and it will be in the next release.

Perhaps you forgot to release it? You don't release it in more than one and a half years.

image

PS.: I even tried directly with the github's async-await branch, which is the default, but I couldn't make it compile. It breaks with the error: failed to select a version for 'pin-project'.

rsalmei commented 2 years ago

The above pin error was a few months ago. I just tried it again now, and it doesn't compile again, for a different reason:

image

PS.: The code it was trying to compile is exactly your Datetime example.

rsalmei commented 2 years ago

Wow, I found it! chrono-tz = "0.5" was the culprit. It did work now with 0.6!!

Well, I would still prefer to use a released version, otherwise, I'll need to pin the specific commit in your repo. But OK, thank you.