Skip to content

Commit 56fd0d9

Browse files
committed
fix(query_row_optional): do not treat rows with NULL as missing rows
Instead of treating NULL type error as absence of the row, handle NULL values with SQL. Previously we sometimes accidentally treated a single column being NULL as the lack of the whole row.
1 parent 9c931c2 commit 56fd0d9

File tree

5 files changed

+40
-15
lines changed

5 files changed

+40
-15
lines changed

src/chat.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,7 +1042,13 @@ impl ChatId {
10421042
pub(crate) async fn get_timestamp(self, context: &Context) -> Result<Option<i64>> {
10431043
let timestamp = context
10441044
.sql
1045-
.query_get_value("SELECT MAX(timestamp) FROM msgs WHERE chat_id=?", (self,))
1045+
.query_get_value(
1046+
"SELECT MAX(timestamp)
1047+
FROM msgs
1048+
WHERE chat_id=?
1049+
HAVING COUNT(timestamp) > 0",
1050+
(self,),
1051+
)
10461052
.await?;
10471053
Ok(timestamp)
10481054
}
@@ -1247,7 +1253,7 @@ impl ChatId {
12471253
) -> Result<Option<(String, String, String)>> {
12481254
self.parent_query(
12491255
context,
1250-
"rfc724_mid, mime_in_reply_to, mime_references",
1256+
"rfc724_mid, mime_in_reply_to, IFNULL(mime_references, '')",
12511257
state_out_min,
12521258
|row: &rusqlite::Row| {
12531259
let rfc724_mid: String = row.get(0)?;
@@ -1401,7 +1407,10 @@ impl ChatId {
14011407
context
14021408
.sql
14031409
.query_get_value(
1404-
"SELECT MAX(timestamp) FROM msgs WHERE chat_id=? AND state!=?",
1410+
"SELECT MAX(timestamp)
1411+
FROM msgs
1412+
WHERE chat_id=? AND state!=?
1413+
HAVING COUNT(*) > 0",
14051414
(self, MessageState::OutDraft),
14061415
)
14071416
.await?
@@ -1417,7 +1426,10 @@ impl ChatId {
14171426
context
14181427
.sql
14191428
.query_get_value(
1420-
"SELECT MAX(timestamp) FROM msgs WHERE chat_id=? AND hidden=0 AND state>?",
1429+
"SELECT MAX(timestamp)
1430+
FROM msgs
1431+
WHERE chat_id=? AND hidden=0 AND state>?
1432+
HAVING COUNT(*) > 0",
14211433
(self, MessageState::InFresh),
14221434
)
14231435
.await?
@@ -4400,7 +4412,10 @@ pub async fn add_device_msg_with_importance(
44004412
if let Some(last_msg_time) = context
44014413
.sql
44024414
.query_get_value(
4403-
"SELECT MAX(timestamp) FROM msgs WHERE chat_id=?",
4415+
"SELECT MAX(timestamp)
4416+
FROM msgs
4417+
WHERE chat_id=?
4418+
HAVING COUNT(*) > 0",
44044419
(chat_id,),
44054420
)
44064421
.await?

src/ephemeral.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,16 @@ impl ChatId {
176176
pub async fn get_ephemeral_timer(self, context: &Context) -> Result<Timer> {
177177
let timer = context
178178
.sql
179-
.query_get_value("SELECT ephemeral_timer FROM chats WHERE id=?;", (self,))
179+
.query_row(
180+
"SELECT IFNULL(ephemeral_timer, 0) FROM chats WHERE id=?",
181+
(self,),
182+
|row| {
183+
let timer: Timer = row.get(0)?;
184+
Ok(timer)
185+
},
186+
)
180187
.await?;
181-
Ok(timer.unwrap_or_default())
188+
Ok(timer)
182189
}
183190

184191
/// Set ephemeral timer value without sending a message.
@@ -509,7 +516,8 @@ async fn next_delete_device_after_timestamp(context: &Context) -> Result<Option<
509516
FROM msgs
510517
WHERE chat_id > ?
511518
AND chat_id != ?
512-
AND chat_id != ?;
519+
AND chat_id != ?
520+
HAVING count(*) > 0
513521
"#,
514522
(DC_CHAT_ID_TRASH, self_chat_id, device_chat_id),
515523
)
@@ -533,7 +541,8 @@ async fn next_expiration_timestamp(context: &Context) -> Option<i64> {
533541
SELECT min(ephemeral_timestamp)
534542
FROM msgs
535543
WHERE ephemeral_timestamp != 0
536-
AND chat_id != ?;
544+
AND chat_id != ?
545+
HAVING count(*) > 0
537546
"#,
538547
(DC_CHAT_ID_TRASH,), // Trash contains already deleted messages, skip them
539548
)

src/message.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ impl MsgId {
222222
pub async fn hop_info(self, context: &Context) -> Result<Option<String>> {
223223
context
224224
.sql
225-
.query_get_value("SELECT hop_info FROM msgs WHERE id=?", (self,))
225+
.query_get_value("SELECT IFNULL(hop_info, '') FROM msgs WHERE id=?", (self,))
226226
.await
227227
}
228228

@@ -1993,7 +1993,9 @@ pub(crate) async fn rfc724_mid_exists_ex(
19931993
.query_row_optional(
19941994
&("SELECT id, timestamp_sent, MIN(".to_string()
19951995
+ expr
1996-
+ ") FROM msgs WHERE rfc724_mid=? ORDER BY timestamp_sent DESC"),
1996+
+ ") FROM msgs WHERE rfc724_mid=?
1997+
HAVING COUNT(*) > 0 -- Prevent MIN(expr) from returning NULL when there are no rows.
1998+
ORDER BY timestamp_sent DESC"),
19971999
(rfc724_mid,),
19982000
|row| {
19992001
let msg_id: MsgId = row.get(0)?;

src/mimefactory.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ impl MimeFactory {
201201
let (in_reply_to, references) = context
202202
.sql
203203
.query_row(
204-
"SELECT mime_in_reply_to, mime_references FROM msgs WHERE id=?",
204+
"SELECT mime_in_reply_to, IFNULL(mime_references, '')
205+
FROM msgs WHERE id=?",
205206
(msg.id,),
206207
|row| {
207208
let in_reply_to: String = row.get(0)?;

src/sql.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -558,15 +558,13 @@ impl Sql {
558558
self.call(move |conn| match conn.query_row(sql.as_ref(), params, f) {
559559
Ok(res) => Ok(Some(res)),
560560
Err(rusqlite::Error::QueryReturnedNoRows) => Ok(None),
561-
Err(rusqlite::Error::InvalidColumnType(_, _, rusqlite::types::Type::Null)) => Ok(None),
562561
Err(err) => Err(err.into()),
563562
})
564563
.await
565564
}
566565

567566
/// Executes a query which is expected to return one row and one
568-
/// column. If the query does not return a value or returns SQL
569-
/// `NULL`, returns `Ok(None)`.
567+
/// column. If the query does not return any rows, returns `Ok(None)`.
570568
pub async fn query_get_value<T>(
571569
&self,
572570
query: &str,

0 commit comments

Comments
 (0)