duckdb / duckdb-rs

Ergonomic bindings to duckdb for Rust
MIT License
464 stars 95 forks source link

Segmentation fault when using appender API #211

Open jubos opened 11 months ago

jubos commented 11 months ago

When using the appender API, you can generate a segfault if you break a foreign key constraint.

If you use the insert API, you will get a returned error when you conduct an insert that breaks a foreign key constraint.

The reproduction of the bug is in this repo: https://github.com/jubos/duckdb-sandbox

You can run the following to generate it:

cargo run test.db
wangfenjin commented 11 months ago

could you try reproduce this issue using the c/c++ API? If reproducible means we should report it in duckdb repo

ctsrc commented 8 months ago

Had a similar thing happen to me when I appended rows with Primary Key values that were already in the table. Instead of returning an error it segfaulted.

Platform is macOS

samuelcolvin commented 5 months ago

I'm getting the same, @wangfenjin @Mause, since @jubos has provided a reproduction of the issue, surely this deserves to be investigated as a priority?

Mause commented 5 months ago

I'm getting the same, @wangfenjin @Mause, since @jubos has provided a reproduction of the issue, surely this deserves to be investigated as a priority?

I've investigated it yeah, but as neither myself nor Wang have merge access, our ability to fix it is limited

samuelcolvin commented 5 months ago

Sorry to ping you, I assumed since you're the most active contributors, you would have merge access.

To be honest, I think #287 is more problematic as that's (afaik) correct usage causing a memory error.

Swoorup commented 5 months ago

I have a branch where I've fixed quite a lot of bugs related to complex/nested types but I am waiting few of my changes to be merged before I can proceed. For now I just have a local copy.

samuelcolvin commented 5 months ago

That's great news @Swoorup, once you have a PR that you think might fix this or #287, let me know and I can test it with my code.

I'm very keen to use duckdb from Rust lots more, but I'd really like the memory errors to get fixed. We (Pydantic, the company) might be able to sponsor significantly if anyone is willing and able to work through these issues.

Swoorup commented 5 months ago

I roughly tested this particular issue in my branch and I get, which I assume is correct?

Failed to flush appender: DuckDBFailure(Error { code: Unknown, extended_code: 1 }, Some("Violates foreign key constraint because key \"id: 0\" does not exist in the referenced table"))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The issue with generated columns is different though I think. It doesn't segfault but panics on mismatched schema, as my approach is first getting the table schema and validating it against what will be appended.

I don't have time atm to polish it up and is quite deviated from this repo fit to my own purpose. I also removed anything else other than the data chunk and arrow for appending. What kind of sponsorships are we talking about? 😛

samuelcolvin commented 4 months ago

What kind of sponsorships are we talking about? 😛

I think we're now looking more at datafusion, but if duckdb becomes a priority again, I'll be in touch.