tursodatabase / libsql

libSQL is a fork of SQLite that is both Open Source, and Open Contributions.
https://turso.tech/libsql
MIT License
9.54k stars 252 forks source link

CHECK failure not propagated on failing INSERT/UPDATE #1655

Closed ignatz closed 1 month ago

ignatz commented 1 month ago

I noticed that insert/update queries do not return an error even when they fail due to violated (column) constraints.

PoC: https://github.com/ignatz/libsql_bench/blob/check/src/libsql.rs (sorry, I recycled a PoC from an unrelated bug and it's using libsql 0.3.5 but I promise that I ran into this on 0.5.0 too :) )

In short, defining a table

CREATE TABLE person (
  id         INTEGER PRIMARY KEY NOT NULL,
  integer    INTEGER CHECK(integer < 42)
);

and running

conn.query("INSERT INTO person (id, integer) VALUES ($1, $2)", (1, 512)).await.unwrap();

I would expect a panic, which it doesn't. Note, this is really just about error propagation. The actual insertion does correctly fail when inspecting the table afterwards.

When I do the same with rusqlite, I get:

called `Result::unwrap()` on an `Err` value: Rusqlite(SqliteFailure(Error { code: ConstraintViolation, extended_code: 275 }, Some("CHECK constraint failed: integer < 42")))

which is what I would expect.

sivukhin commented 1 month ago

Hi @ignatz! You are using query method which return iterator. If you will consume this iterator then it will return the expected error to you on the first attempt.

But in your case you better use exec method instead which will work as you expect.

ignatz commented 1 month ago

You are using query method which return iterator. If you will consume this iterator then it will return the expected error to you on the first attempt.

My bad. That's right :pray:

But in your case you better use exec method instead which will work as you expect.

It will work for this basic example but often I'll want to "INSERT ... RETURNING *". Is there any chance to add a shorthand non-batch version?

let foo : Row = conn.prepare("...").await?.query_row((....)).await?;

Anyway, this is very much a tangent now. Feel free to close and thanks again