Skip to content

Commit 2e2e5db

Browse files
committed
Add optimized view for fetching room moderators
Using the user_permissions view for this is very slow: because it has to check each user.moderator value in the various COALESCEs it ends up doing a full scan of the `users` table. This adds a new `room_moderators` view for fetching moderator lists that is a couple of orders of magnitude faster (by constructing the query differently).
1 parent 55232c6 commit 2e2e5db

File tree

6 files changed

+189
-12
lines changed

6 files changed

+189
-12
lines changed

sogs/migrations/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
new_columns,
1212
new_tables,
1313
room_accessible,
14+
room_moderators,
1415
seqno_etc,
1516
user_permissions,
1617
user_perm_futures,
@@ -42,6 +43,7 @@ def migrate(conn, *, check_only=False):
4243
message_views,
4344
user_perm_futures,
4445
room_accessible,
46+
room_moderators,
4547
user_permissions,
4648
file_message,
4749
import_hacks,

sogs/migrations/room_moderators.py

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import logging
2+
from .exc import DatabaseUpgradeRequired
3+
4+
5+
def migrate(conn, *, check_only):
6+
"""
7+
Adds the room_moderators view and drops/replaces the user_permission_overrides_public_mods
8+
index.
9+
"""
10+
11+
from .. import db
12+
13+
if 'room_moderators' in db.metadata.tables:
14+
return False
15+
16+
logging.warning("DB migration: create room_moderators view")
17+
if check_only:
18+
raise DatabaseUpgradeRequired("Create room_moderators view")
19+
20+
if db.engine.name == "sqlite":
21+
conn.execute(
22+
"""
23+
CREATE VIEW room_moderators AS
24+
SELECT session_id, mods.* FROM (
25+
SELECT
26+
room,
27+
"user",
28+
MAX(visible_mod) & 1 AS visible_mod,
29+
MAX(admin) AS admin,
30+
MAX(room_moderator) AS room_moderator,
31+
MAX(global_moderator) AS global_moderator
32+
FROM (
33+
SELECT
34+
room,
35+
"user",
36+
CASE WHEN visible_mod THEN 3 ELSE 2 END AS visible_mod,
37+
admin,
38+
TRUE AS room_moderator,
39+
FALSE AS global_moderator
40+
FROM user_permission_overrides WHERE moderator
41+
42+
UNION ALL
43+
44+
SELECT
45+
rooms.id AS room,
46+
users.id as "user",
47+
CASE WHEN visible_mod THEN 1 ELSE 0 END AS visible_mod,
48+
admin,
49+
FALSE as room_moderator,
50+
TRUE as global_moderator
51+
FROM users CROSS JOIN rooms WHERE moderator
52+
) m GROUP BY "user", room
53+
) mods JOIN users on "user" = users.id
54+
"""
55+
)
56+
else: # postgres
57+
conn.execute(
58+
"""
59+
CREATE VIEW room_moderators AS
60+
SELECT session_id, mods.* FROM (
61+
SELECT
62+
room,
63+
"user",
64+
CAST(MAX(visible_mod) & 1 AS BOOLEAN) AS visible_mod,
65+
bool_or(admin) AS admin,
66+
bool_or(room_moderator) AS room_moderator,
67+
bool_or(global_moderator) AS global_moderator
68+
FROM (
69+
SELECT
70+
room,
71+
"user",
72+
CASE WHEN visible_mod THEN 3 ELSE 2 END AS visible_mod,
73+
admin,
74+
TRUE AS room_moderator,
75+
FALSE AS global_moderator
76+
FROM user_permission_overrides WHERE moderator
77+
78+
UNION ALL
79+
80+
SELECT
81+
rooms.id AS room,
82+
users.id as "user",
83+
CASE WHEN visible_mod THEN 1 ELSE 0 END AS visible_mod,
84+
admin,
85+
FALSE as room_moderator,
86+
TRUE as global_moderator
87+
FROM users CROSS JOIN rooms WHERE moderator
88+
) m GROUP BY "user", room
89+
) mods JOIN users on "user" = users.id
90+
"""
91+
)
92+
93+
conn.execute("DROP INDEX IF EXISTS user_permission_overrides_public_mods")
94+
conn.execute(
95+
"CREATE INDEX IF NOT EXISTS user_permission_overrides_mods "
96+
"ON user_permission_overrides(room) WHERE moderator"
97+
)
98+
99+
return True

sogs/model/room.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -981,11 +981,12 @@ def get_mods(self, user=None):
981981
([public_mods], [public_admins], [hidden_mods], [hidden_admins])
982982
"""
983983

984+
visible_clause = "" if self.check_moderator(user) else "AND visible_mod"
984985
m, hm, a, ha = [], [], [], []
985986
for session_id, visible, admin in query(
986-
"""
987-
SELECT session_id, visible_mod, admin FROM user_permissions
988-
WHERE room = :r AND moderator
987+
f"""
988+
SELECT session_id, visible_mod, admin FROM room_moderators
989+
WHERE room = :r {visible_clause}
989990
ORDER BY session_id
990991
""",
991992
r=self.id,
@@ -996,9 +997,6 @@ def get_mods(self, user=None):
996997

997998
((a if admin else m) if visible else (ha if admin else hm)).append(session_id)
998999

999-
if user is None or not any(user.session_id in modlist for modlist in (m, hm, a, ha)):
1000-
hm, ha = [], []
1001-
10021000
return m, a, hm, ha
10031001

10041002
def get_all_moderators(self):

sogs/schema.pgsql

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,7 @@ CREATE TABLE user_permission_overrides (
240240
PRIMARY KEY(room, "user"),
241241
CHECK(NOT (banned AND (moderator OR admin))) /* Mods/admins cannot be banned */
242242
);
243-
CREATE INDEX user_permission_overrides_public_mods ON
244-
user_permission_overrides(room) WHERE moderator OR admin;
243+
CREATE INDEX user_permission_overrides_mods ON user_permission_overrides(room) WHERE moderator;
245244

246245
-- Create a trigger to maintain the implication "admin implies moderator"
247246
CREATE OR REPLACE FUNCTION trigger_user_perms_admins_are_mods()
@@ -360,7 +359,8 @@ EXECUTE PROCEDURE trigger_room_metadata_pinned_remove();
360359

361360
-- View of permissions; for users with an entry in user_permissions we use those values; for null
362361
-- values or no user_permissions entry we return the room's default read/write values (and false for
363-
-- the other fields).
362+
-- the other fields). This view should only be used for querying individual user permissions as it
363+
-- will involve a full table scan on `users` if not given a "user" value in the query.
364364
CREATE VIEW user_permissions AS
365365
SELECT
366366
rooms.id AS room,
@@ -388,6 +388,44 @@ FROM
388388
users CROSS JOIN rooms LEFT OUTER JOIN user_permission_overrides ON
389389
(users.id = user_permission_overrides."user" AND rooms.id = user_permission_overrides.room);
390390

391+
-- Used to accesses the moderator list for a room. This view is considerably faster than querying
392+
-- the `user_permissions` table for a list of all moderators.
393+
CREATE VIEW room_moderators AS
394+
SELECT session_id, mods.* FROM (
395+
SELECT
396+
room,
397+
"user",
398+
-- visible_mod gets priority from the per-room row if it exists, so we use 3/2 for the
399+
-- per-room value below, 1/0 for the global value, take the max, then look for an odd value
400+
-- to give us the visibility bit:
401+
CAST(MAX(visible_mod) & 1 AS BOOLEAN) AS visible_mod,
402+
bool_or(admin) AS admin,
403+
bool_or(room_moderator) AS room_moderator,
404+
bool_or(global_moderator) AS global_moderator
405+
FROM (
406+
SELECT
407+
room,
408+
"user",
409+
CASE WHEN visible_mod THEN 3 ELSE 2 END AS visible_mod,
410+
admin,
411+
TRUE AS room_moderator,
412+
FALSE AS global_moderator
413+
FROM user_permission_overrides WHERE moderator
414+
415+
UNION ALL
416+
417+
SELECT
418+
rooms.id AS room,
419+
users.id as "user",
420+
CASE WHEN visible_mod THEN 1 ELSE 0 END AS visible_mod,
421+
admin,
422+
FALSE as room_moderator,
423+
TRUE as global_moderator
424+
FROM users CROSS JOIN rooms WHERE moderator
425+
) m GROUP BY "user", room
426+
) mods JOIN users on "user" = users.id;
427+
428+
391429
-- Scheduled changes to user permissions. For example, to implement a 2-day timeout you would set
392430
-- their user_permissions.write to false, then set a `write = true` entry with a +2d timestamp here.
393431
-- Or to implement a join delay you could set room defaults to false then insert a value here to be

sogs/schema.sqlite

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,7 @@ CREATE TABLE user_permission_overrides (
215215
PRIMARY KEY(room, user),
216216
CHECK(NOT (banned AND (moderator OR admin))) /* Mods/admins cannot be banned */
217217
) WITHOUT ROWID;
218-
CREATE INDEX user_permission_overrides_public_mods ON
219-
user_permission_overrides(room) WHERE moderator OR admin;
218+
CREATE INDEX user_permission_overrides_mods ON user_permission_overrides(room) WHERE moderator;
220219

221220
-- Create a trigger to maintain the implication "admin implies moderator"
222221
CREATE TRIGGER user_perms_update_admins_are_mods AFTER UPDATE OF moderator, admin ON user_permission_overrides
@@ -314,7 +313,8 @@ END;
314313

315314
-- View of permissions; for users with an entry in user_permissions we use those values; for null
316315
-- values or no user_permissions entry we return the room's default read/write values (and false for
317-
-- the other fields).
316+
-- the other fields). This view should only be used for querying individual user permissions as it
317+
-- will involve a full table scan on `users` if not given a "user" value in the query.
318318
CREATE VIEW user_permissions AS
319319
SELECT
320320
rooms.id AS room,
@@ -342,6 +342,43 @@ FROM
342342
users JOIN rooms LEFT OUTER JOIN user_permission_overrides ON
343343
(users.id = user_permission_overrides.user AND rooms.id = user_permission_overrides.room);
344344

345+
-- Used to accesses the moderator list for a room. This view is considerably faster than querying
346+
-- the `user_permissions` table for a list of all moderators.
347+
CREATE VIEW room_moderators AS
348+
SELECT session_id, mods.* FROM (
349+
SELECT
350+
room,
351+
"user",
352+
-- visible_mod gets priority from the per-room row if it exists, so we use 3/2 for the
353+
-- per-room value below, 1/0 for the global value, take the max, then look for an odd value
354+
-- to give us the visibility bit:
355+
MAX(visible_mod) & 1 AS visible_mod,
356+
MAX(admin) AS admin,
357+
MAX(room_moderator) AS room_moderator,
358+
MAX(global_moderator) AS global_moderator
359+
FROM (
360+
SELECT
361+
room,
362+
"user",
363+
CASE WHEN visible_mod THEN 3 ELSE 2 END AS visible_mod,
364+
admin,
365+
TRUE AS room_moderator,
366+
FALSE AS global_moderator
367+
FROM user_permission_overrides WHERE moderator
368+
369+
UNION ALL
370+
371+
SELECT
372+
rooms.id AS room,
373+
users.id as "user",
374+
CASE WHEN visible_mod THEN 1 ELSE 0 END AS visible_mod,
375+
admin,
376+
FALSE as room_moderator,
377+
TRUE as global_moderator
378+
FROM users CROSS JOIN rooms WHERE moderator
379+
) m GROUP BY "user", room
380+
) mods JOIN users on "user" = users.id;
381+
345382
-- Scheduled changes to user permissions. For example, to implement a 2-day timeout you would set
346383
-- their user_permissions.write to false, then set a `write = true` entry with a +2d timestamp here.
347384
-- Or to implement a join delay you could set room defaults to false then insert a value here to be

tests/test_blinding.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,9 @@ def test_blinded_transition(
247247

248248
assert unmigrated == set()
249249

250+
for r in (room, room2, r3):
251+
r._refresh(perms=True)
252+
250253
# NB: "global_admin" isn't actually an admin anymore (we transferred the permission to the
251254
# blinded equivalent), so shouldn't see the invisible mods:
252255
assert room.get_mods(global_admin) == ([mod.blinded_id], [admin.blinded_id], [], [])

0 commit comments

Comments
 (0)