"Invalid memory reference" when appending a row with a timestamp

[danbruder]

Thanks for the crate!

I'm trying to use the Appender api but get a segfault when there is a timestamp in the row. Here's a minimal example showing what happens (and here is a repo reproducing it):

    #[test]
    fn timestamp_appender_minimal_example_sig_segv() {
        let db = Connection::open_in_memory().unwrap();

        let create_table_sql = r"
          CREATE TABLE item (
              id INTEGER NOT NULL,
              ts TIMESTAMP
          );";
        db.execute_batch(create_table_sql).unwrap();

        let mut app = db.appender("item").unwrap();
        let row_count = 10;
        for i in 0..row_count {
            app.append_row(params![i, "1970-01-01T00:00:00Z"]).unwrap();
        }

        // running 1 test
        // error: test failed, to rerun pass '--lib'
        // Caused by:
        //   process didn't exit successfully: `/path-to-repo` (signal: 11, SIGSEGV: invalid memory reference)

        let val = db
            .query_row("SELECT count(1) FROM item", [], |row| {
                <(u32,)>::try_from(row)
            })
            .unwrap();

        assert_eq!(val, (row_count,));
    }

I've enabled the chrono feature and tried with Utc::now as well. This is using the bundled version of duckdb.

I'd love to help fix the issue, would you mind pointing me in the right direction? I started a draft with failing tests here

[wangfenjin]

Hi @danbruder , thanks for the report! The root cause is currently we don't support bind timestamp param. It will not be an easy fix, but I'll give you some clue:

1. The most important change we need is to add support binding timestamp in here

// this is simplified, we need to convert i according to timeunit u
ValueRef::Timestamp(u, i) => unsafe { ffi::duckdb_append_timestamp(ptr, ffi::duckdb_timestamp{micros: i}) }

2. Then we need support convert some rust data types into Value type
3. And also need to make this type impl ToSql

We may change chrono.rs convert to Timestamp, currently it use String / Text

[wangfenjin]

Also I create an issue in duckdb/duckdb#3402

If we can support append string as timestamp it will make us easier

[hawkfish]

It looks like the DuckDB C API appender does not cast strings to timestamps internally (it will try to cast it to an int64_t).