Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

query_as returning Option<T> when using 'returning' syntax on sqlite #2407

Closed
arlyon opened this issue Mar 16, 2023 · 1 comment · Fixed by #2409
Closed

query_as returning Option<T> when using 'returning' syntax on sqlite #2407

arlyon opened this issue Mar 16, 2023 · 1 comment · Fixed by #2409
Labels

Comments

@arlyon
Copy link
Contributor

arlyon commented Mar 16, 2023

Bug Description

Variables bound using query_as, are being returned as an Option<T>, leading to type errors. Inserting the variable directly into the query returns a T. No type errors.

Minimal Reproduction

bad:

struct Tweet {
    id: i64,
    text: String,
    is_sent: bool,
    owner_id: Option<i64>,
}

let x = sqlx::query_as!(
    Tweet,
    "INSERT INTO tweet (id, text) VALUES (10, $1) RETURNING *",
    "Hello"
)
.fetch_one(&mut conn)
.await?;
194 |       let x = sqlx::query_as!(
    |  _____________^
195 | |         Tweet,
196 | |         "INSERT INTO tweet (id, text) VALUES (10, $1) RETURNING *",
197 | |         10
198 | |     )
    | |_____^ expected struct `String`, found enum `Option`

ok:

let x = sqlx::query_as!(
    Tweet,
    "INSERT INTO tweet (id, text) VALUES (10, 'Hello') RETURNING *",
)
.fetch_one(&mut conn)
.await?;

Info

  • SQLx version: 0.6.2
  • SQLx features enabled: sqlite, tokio runtime
  • Database server and version: sqlite, latest
  • Operating system: linux (fedora 37)
  • rustc --version: 1.68.0
@arlyon arlyon added the bug label Mar 16, 2023
@arlyon
Copy link
Contributor Author

arlyon commented Mar 16, 2023

After some digging, it seems that describe doesn't return any information to the macros about whether a given column is nullable because it is genuinely nullable or because it is in a prepared statement. SQLx takes all variables from sqlite and unconditionally determines that they are nullable. we know later on from our queries that his is not the case, but have no way of overriding that.

OP_VARIABLE => {
// r[p2] = <value of variable>
state.r.insert(p2, RegDataType::Single(ColumnType::null()));
}

I can see from the explain that there is a decent amount of additional metadata here:

addr  opcode         p1    p2    p3    p4             p5  comment      
----  -------------  ----  ----  ----  -------------  --  -------------
0     Init           0     35    0                    0   Start at 35
1     OpenWrite      0     2     0     4              0   root=2 iDb=0; tweet
2     OpenWrite      1     3     0     k(1,)          0   root=3 iDb=0; sqlite_autoindex_tweet_1
3     Integer        10    2     0                    0   r[2]=10
4     Variable       1     3     0     $1             0   r[3]=parameter(1,$1)
5     NewRowid       0     1     0                    0   r[1]=rowid
6     HaltIfNull     1299  2     2     tweet.id       1   if r[2]=null halt
7     HaltIfNull     1299  2     3     tweet.text     1   if r[3]=null halt
8     HaltIfNull     1299  2     4     tweet.is_sent  1   if r[4]=null halt
9     Affinity       2     4     0     DBCD           0   affinity(r[2..5])
10    Noop           0     0     0                    0   prep index sqlite_autoindex_tweet_1
11    SCopy          2     7     0                    0   r[7]=r[2]; id
12    IntCopy        1     8     0                    0   r[8]=r[1]; rowid
13    MakeRecord     7     2     6                    0   r[6]=mkrec(r[7..8]); for sqlite_autoindex_tweet_1
14    NoConflict     1     16    7     1              0   key=r[7]
15    Halt           1555  2     0     tweet.id       2   
16    MakeRecord     2     4     9                    0   r[9]=mkrec(r[2..5])
17    IdxInsert      1     6     7     1              16  key=r[6]
18    Insert         0     9     1     tweet          57  intkey=r[1] data=r[9]
19    SCopy          2     11    0                    0   r[11]=r[2]
20    SCopy          3     12    0                    0   r[12]=r[3]
21    SCopy          4     13    0                    0   r[13]=r[4]
22    SCopy          5     14    0                    0   r[14]=r[5]
23    MakeRecord     11    4     15                   0   r[15]=mkrec(r[11..14])
24    NewRowid       3     16    0                    0   r[16]=rowid
25    Insert         3     15    16                   0   intkey=r[16] data=r[15]
26    FkCheck        0     0     0                    0   
27    Rewind         3     34    0                    0   
28      Column         3     0     11                   0   r[11]= cursor 3 column 0
29      Column         3     1     12                   0   r[12]= cursor 3 column 1
30      Column         3     2     13                   0   r[13]= cursor 3 column 2
31      Column         3     3     14                   0   r[14]= cursor 3 column 3
32      ResultRow      11    4     0                    0   output=r[11..14]
33    Next           3     28    0                    0   
34    Halt           0     0     0                    0   
35    Transaction    0     1     6     0              1   usesStmtJournal=1
36    Integer        1     4     0                    0   r[4]=1
37    Null           0     5     0                    0   r[5]=NULL
38    OpenEphemeral  3     4     0                    0   nColumn=4
39    Goto           0     1     0                    0   

In particular, register 3 is explicitly checked with HaltIfNull just a few lines down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant