Fix get_filtered_current_state_ids()'s usage of filtered_types

[ara4n]

We weren't applying the "AND type<>?" sql clause to every query, as the query was split up into separate ones.

In practice this didn't break anything as nothing currently uses filtered_types in get_filtered_current_state_ids; i just spotted it as a thinko.

[richvdh]

lgtm I guess. I'm concerned that there isn't a unit test that picked this up though, particularly given we've made a mess of very similar bits of code elsewhere in the past and it's got all the way out to release before we've noticed - any chance of adding one?

[richvdh]

it's not obvious to me that this needs copying: why not just additional_args = unique_types ?

[erikjohnston]

I'm not really sure I'm following what the query looks like here. Say we call this with:

store.get_filtered_current_state_ids(
    room_id="!foo:bar.com",
    types=[("m.room.members", "@bob:foo.com"), ("m.room.topic", "")],
    filtered_types=["m.room.members"]
)

Would produce two queries:

SELECT type, state_key, event_id FROM current_state_events
WHERE room_id = '!foo:bar.com'
AND type = 'm.room.member' AND state_key = '@bob:foo.com' AND type != 'm.room.member'

SELECT type, state_key, event_id FROM current_state_events
WHERE room_id = '!foo:bar.com'
AND type = 'm.room.topic' AND state_key = '' AND type != 'm.room.member'

Which don't make sense? I feel like I'm missing something?

[richvdh]

Erik makes an excellent point. It doesn't make sense. The docstring on the function is a bit terse; to quote the longer one from some other places:

            filtered_types(list[str]|None): Only apply filtering via `types` to this
                list of event types.  Other types of events are returned unfiltered.
                If None, `types` filtering is applied to all events.

That makes it much clearer what filtered_types is supposed to do. To take Erik's example, the queries we actually want are therefore:

SELECT type, state_key, event_id FROM current_state_events
WHERE room_id = '!foo:bar.com'
AND type = 'm.room.member' AND state_key = '@bob:foo.com';

SELECT type, state_key, event_id FROM current_state_events
WHERE room_id = '!foo:bar.com'
AND type = 'm.room.topic' AND state_key = '';

SELECT type, state_key, event_id FROM current_state_events
WHERE room_id = '!foo:bar.com'
AND type != 'm.room.member';

(it doesn't really make much sense to specify ("m.room.topic", "") in types if you are then going to include everything that isn't an m.room.member via filtered_types)

Anyway, I think the above is what the code does currently. In short, wtf, @ara4n ?

[not for the first time, the types vs filtered_types thing is hella confusing, and I wish I had come up with a better system.]

[richvdh]

this looks wrong, as per the above

[ara4n]

@erikjohnston am I right in thinking that your rewrite of the filtered_types hell renders this irrelevant?

[hawkowl]

From my reading, it appears so.