PostgreSQL deferred FK constraint violations are silently dropped

[bitglue]

sqlx 0.5.5
postgresql 11.6

sqlx reports no error when a deferred FK constraint is violated.

For example, create two tables with a FK constraint between them, and see that with psql, an error is reported:

postgres=# create table foo (foo text primary key);
CREATE TABLE
postgres=# create table bar (foo text references foo deferrable initially deferred);
CREATE TABLE
postgres=# insert into bar values ('beep');
ERROR:  insert or update on table "bar" violates foreign key constraint "bar_foo_fkey"
DETAIL:  Key (foo)=(beep) is not present in table "foo".

Now, try performing this insert with sqlx:

#[derive(Debug, sqlx::FromRow)]
struct Bar {
    foo: String,
}

#[async_std::test]
async fn deferred_fk() {
    let pool = common::connection_pool().await;
    let bar: Bar = sqlx::query_as("insert into bar values ('foo') returning *")
        .fetch_one(&pool)
        .await
        .unwrap();
    println!("bar successfully inserted: {:?}", bar);
}

I'd think this should panic because the constraint was violated, but it does not:

$ cargo test deferred_fk
    Finished test [unoptimized + debuginfo] target(s) in 0.38s
[...]
     Running tests/db.rs (target/debug/deps/db-1bbe88be67e83ab9)

running 1 test
test deferred_fk ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 17 filtered out; finished in 0.03s

I'm guessing the problem is sqlx sees the data row, then ignores the error in the response from the server:

Frame 23: 329 bytes on wire (2632 bits), 329 bytes captured (2632 bits)
Null/Loopback
Internet Protocol Version 6, Src: ::1, Dst: ::1
Transmission Control Protocol, Src Port: 5434, Dst Port: 49930, Seq: 402, Ack: 264, Len: 253
PostgreSQL
    Type: Bind completion
    Length: 4
PostgreSQL
    Type: Data row     <---- the row was inserted
    Length: 13
    Field count: 1
        Column length: 3
        Data: 666f6f
PostgreSQL
    Type: Portal suspended
    Length: 4
PostgreSQL
    Type: Error     <---- but the transaction failed to commit
    Length: 222
    Severity: ERROR
    Text: ERROR
    Code: 23503
    Message: insert or update on table "bar" violates foreign key constraint "bar_foo_fkey"
    Detail: Key (foo)=(foo) is not present in table "foo".
    Schema: public
    Table: bar
    Constraint: bar_foo_fkey
    File: ri_triggers.c
    Line: 2783
    Routine: ri_ReportViolation
PostgreSQL
    Type: Ready for query
    Length: 5
    Status: Idle (73)

The above is from this pcap: sqlx.pcap.gz

The error is reported this way because the constraint is deferred, so the row can be inserted, but then on commit (which happens automatically) the constraint is violated and the transaction aborts. So I'd also wonder if there is a larger class of bugs where a statement succeeds but sqlx then misses errors from autocommitting the transaction.

[abonander]

It looks like the fix should be pretty simple, since we're setting the limit parameter to 1 in the Execute message, we just need to wait for the stream to yield None here (meaning we received ReadyForQuery) before returning, and if there's an error we'll get it at that point:

https://github.com/launchbadge/sqlx/blob/master/sqlx-core/src/postgres/connection/executor.rs#L378-L382

However, if we do generate an error, what do we do with the row that we already got? Should we change the return type of .fetch_one() and .fetch_optional() to return a result like this:

pub type FetchResult<T> = Result<T, FetchError<T>>;

pub struct FetchError<T> {
    pub error: sqlx_core::Error,
    pub row: Option<T>
}

// should allow existing usages of `?` to continue working
impl From<T> From<FetchError<T>> for sqlx_core::Error {}

[abonander]

As a workaround, you can use .fetch() instead of .fetch_one() and poll it twice:

let mut stream = sqlx::query_as("insert into bar values ('foo') returning *").fetch(&pool);

let bar: Bar = stream.try_next().await?.ok_or(sqlx::Error::RowNotFound)?;
// check for deferred FK constraint error
stream.try_next().await?;

[bitglue]

fwiw, I can't personally think of a real use case where having the row that wasn't inserted is useful. Nor does psql display it:

postgres=# create table foo (foo text primary key);
CREATE TABLE
Time: 662.821 ms
postgres=# create table bar (foo text references foo deferrable initially deferred);
CREATE TABLE
Time: 20.017 ms
postgres=# insert into bar values ('beep') returning *;
ERROR:  insert or update on table "bar" violates foreign key constraint "bar_foo_fkey"
DETAIL:  Key (foo)=(beep) is not present in table "foo".
Time: 40.422 ms

I think the complexity of having a type that can represent success-but-also-failure would be complexity without material benefit, and I'd rather just get an error in this case.

[bitglue]

furthermore, if someone has an unusual use case where this information really is relevant, they can use .fetch as you suggest in the workaround, or they can use an explicit transaction. Using fetch_one means to me "i want exactly one row, or an error."

[kurtbuilds]

I just ran into this issue. Is there a plan for fixing .fetch_one()? Right now, .fetch_one() will return data that isn't committed to the database, making the insertion look like success, which is terribly confusing.

Created a reproduction repo here:

https://github.com/kurtbuilds/fetch-one-bug