fix: markseen_msgs(): Mark reactions to specified messages as seen too (#7884)

[iequidoo]

This allows to remove notifications for reactions from other devices. NB: UIs should pass all messages to markseen_msgs(), incl. outgoing ones. markseen_msgs() should be called when a message comes into view or when a reaction for a message being in view arrives.

Also don't emit MsgsNoticed from receive_imf_inner() if the chat still contains fresh hidden messages, i.e. include reactions into this logic, to avoid removing notifications for reactions until they are seen on another device.

Close #7884

[r10s]

NB: UIs should pass all messages to markseen_msgs(), incl. outgoing ones. markseen_msgs() should be called when a message comes into view or when a reaction for a message being in view arrives.

i checked android and iOS, here since forever, markseen_msgs is called unconditionally, so also for outgoing ones.

@nicodh can you check that for desktop as well?

@iequidoo i think, this information should be added to the docs as well, smth as "as markseen_msgs() is used also for synchronisation and to track last position, it should be called unconditionally for all messages that scroll into view, including outgoing and status messages" (or whatever the exact reason is)

[iequidoo]

@iequidoo i think, this information should be added to the docs as well, smth as "as markseen_msgs() is used also for synchronisation and to track last position, it should be called unconditionally for all messages that scroll into view, including outgoing and status messages" (or whatever the exact reason is)

Done

[link2xt]

Can query_map_vec be used?

[iequidoo]

Unfortunately, we need many SQL queries here because different id and rfc724_mid are passed in each query, so it won't help a lot, we'll still need to join the returned vectors.

[link2xt]

I don't understand what hidden=0 OR hidden=1 means. Can this be removed? Is it a way to check for NULL? Does NULL ever get inserted?

[iequidoo]

It's a way to employ msgs_index7 ON msgs (state, hidden, chat_id). Unfortunately, SQLite doesn't track all values used for a particular column, so it can't use the index because it thinks that it should iterate over all messages having state specified before. I made some experiments earlier that confirm this behavior, though i didn't repeat them for this PR.

But it seems that i made a mistake here, OR should be the final operator connecting AND-terms. The relevant docs are here: https://sqlite.org/queryplanner.html#_or_connected_terms_in_the_where_clause. Better to use hidden IN (0,1), it defenitely works well with the index because i recently measured it in 677a799.

[link2xt]

I have used EXPLAIN QUERY PLAN in #8040, it is useful for showing which index is actually used. As long as you never run ANALYZE sqlite is stable (#6585)

[iequidoo]

EXPLAIN QUERY PLAN UPDATE msgs SET state=? WHERE state=? AND chat_id=? AND timestamp<?;
QUERY PLAN
`--SEARCH msgs USING INDEX msgs_index7 (state=?)
EXPLAIN QUERY PLAN UPDATE msgs SET state=? WHERE state=? AND hidden IN (0,1) AND chat_id=? AND timestamp<?;
QUERY PLAN
`--SEARCH msgs USING INDEX msgs_index7 (state=? AND hidden=? AND chat_id=? AND timestamp<?)

[iequidoo]

I ran it on my db, so it employs also timestamp because i'm testing some locally added db migrations. But anyway, w/o hidden IN (0,1) it can't even use chat_id.

[iequidoo]

Maybe we already add timestamp to msgs_index7? I see many places where it would be useful, e.g. calc_sort_timestamp(), building chatlists etc.