Improved detection of nullability for postgres

[jyn514]

This query currently returns an Option<i64>. It would be nice to instead return i64, because COUNT is never nullable.

async fn f() -> Result<i64, sqlx::Error> {
    sqlx::query!(
        "SELECT COUNT(*) FROM crates",
    )
    .fetch_one(unimplemented!())
    .await
    .map(|rec| rec.count)
}

error[E0308]: mismatched types
  --> src/main.rs:6:5
   |
6  | /     sqlx::query!(
7  | |         "SELECT COUNT(*) FROM crates",
8  | |     )
9  | |     .fetch_one(unimplemented!())
10 | |     .await
11 | |     .map(|rec| rec.count)
   | |_________________________^ expected `i64`, found enum `std::option::Option`
   |
   = note: expected enum `std::result::Result<i64, _>`
              found enum `std::result::Result<std::option::Option<i64>, _>`

@mehcode mentioned that I can give SQLx a type hint by using r#"SELECT COUNT(*) as "count!" FROM crates;"#; this is a good workaround in the meantime but I'd love for sqlx to be able to figure it out itself. A possible implementation is to select from a temporary table or view and get the nullability from that view.

[jyn514]

Another (related?) issue: sqlx thinks this is an Option<i32>, not an i32.

query!(
        "INSERT INTO doc_coverage (release_id, total_items, documented_items)
            VALUES ($1, $2, $3)
            ON CONFLICT (release_id) DO UPDATE
                SET
                    total_items = $2,
                    documented_items = $3
            RETURNING release_id",
        release_id,
        doc_coverage.total_items,
        doc_coverage.documented_items,
    )
    .fetch_one(conn)
    .block()?
    .release_id

[abonander]

@jyn514 can you provide a explain verbose for that INSERT INTO ... RETURNING release_id query? A minimal reproduction with schema would be helpful for testing.

We can feasibly detect simple cases like count(*) but arbitrary expressions are another matter. At the very least we would need a basic expression tree parser and to hardcode knowledge of various functions and their outputs. There's nothing in pg_proc that tells us definitively whether a function may or may not return null.

[jyn514]

cratesfyi=# explain verbose SELECT COUNT(*) FROM crates;
                             QUERY PLAN                              
---------------------------------------------------------------------
 Aggregate  (cost=10.88..10.88 rows=1 width=8)
   Output: count(*)
   ->  Seq Scan on public.crates  (cost=0.00..10.70 rows=70 width=0)
(3 rows)

cratesfyi=# explain verbose INSERT INTO doc_coverage (release_id, total_items, documented_items)
cratesfyi-#             VALUES (1, 10, 7)
cratesfyi-#             ON CONFLICT (release_id) DO UPDATE
cratesfyi-#                 SET
cratesfyi-#                     total_items = 10,
cratesfyi-#                     documented_items = 7
cratesfyi-#             RETURNING release_id;
                            QUERY PLAN                            
------------------------------------------------------------------
 Insert on public.doc_coverage  (cost=0.00..0.01 rows=1 width=20)
   Output: doc_coverage.release_id
   Conflict Resolution: UPDATE
   Conflict Arbiter Indexes: doc_coverage_release_id_key
   ->  Result  (cost=0.00..0.01 rows=1 width=20)
         Output: 1, 10, 7, NULL::integer, NULL::integer
(6 rows)

cratesfyi=# \d doc_coverage
                       Table "public.doc_coverage"
            Column            |  Type   | Collation | Nullable | Default 
------------------------------+---------+-----------+----------+---------
 release_id                   | integer |           |          | 
 total_items                  | integer |           |          | 
 documented_items             | integer |           |          | 
 total_items_needing_examples | integer |           |          | 
 items_with_examples          | integer |           |          | 
Indexes:
    "doc_coverage_release_id_key" UNIQUE CONSTRAINT, btree (release_id)
Foreign-key constraints:
    "doc_coverage_release_id_fkey" FOREIGN KEY (release_id) REFERENCES releases(id)

There's nothing in pg_proc that tells us definitively whether a function may or may not return null.

That's unfortunate ... I don't want to make you add a full parser (I know one of the goals of the proc-macros is to avoid having to do that), so if this is too hard to support it's not a blocker for us.