From f4beac5fd898ccd585a5f1b226b6b4add0eebe4f Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Mon, 13 Dec 2021 18:34:48 +0000 Subject: [PATCH 1/2] Remove redundant `COALESCE()`s around `COUNT()`s in database queries `COUNT()` never returns `NULL`. A `COUNT(*)` over 0 rows is 0 and a `COUNT(NULL)` is also 0. --- changelog.d/11567.misc | 1 + .../storage/databases/main/event_federation.py | 4 ++-- synapse/storage/databases/main/metrics.py | 18 +++++++++--------- .../databases/main/monthly_active_users.py | 4 ++-- synapse/storage/databases/main/registration.py | 4 ++-- synapse/storage/databases/main/relations.py | 2 +- synapse/storage/databases/main/room.py | 2 +- synapse/storage/databases/main/stats.py | 2 +- tests/storage/test_event_federation.py | 2 +- 9 files changed, 20 insertions(+), 19 deletions(-) create mode 100644 changelog.d/11567.misc diff --git a/changelog.d/11567.misc b/changelog.d/11567.misc new file mode 100644 index 000000000000..d9af8bdb0537 --- /dev/null +++ b/changelog.d/11567.misc @@ -0,0 +1 @@ +Remove redundant `COALESCE()`s around `COUNT()`s in database queries. diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index 2287f1cc6877..0c5ca506d427 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -1393,7 +1393,7 @@ async def prune_staged_events_in_room( count = await self.db_pool.simple_select_one_onecol( table="federation_inbound_events_staging", keyvalues={"room_id": room_id}, - retcol="COALESCE(COUNT(*), 0)", + retcol="COUNT(*)", desc="prune_staged_events_in_room_count", ) @@ -1486,7 +1486,7 @@ async def _get_stats_for_federation_staging(self): def _get_stats_for_federation_staging_txn(txn): txn.execute( - "SELECT coalesce(count(*), 0) FROM federation_inbound_events_staging" + "SELECT count(*) FROM federation_inbound_events_staging" ) (count,) = txn.fetchone() diff --git a/synapse/storage/databases/main/metrics.py b/synapse/storage/databases/main/metrics.py index 3bb21958d1e9..1480a0f04829 100644 --- a/synapse/storage/databases/main/metrics.py +++ b/synapse/storage/databases/main/metrics.py @@ -105,7 +105,7 @@ async def count_daily_e2ee_messages(self): def _count_messages(txn): sql = """ - SELECT COALESCE(COUNT(*), 0) FROM events + SELECT COUNT(*) FROM events WHERE type = 'm.room.encrypted' AND stream_ordering > ? """ @@ -122,7 +122,7 @@ def _count_messages(txn): like_clause = "%:" + self.hs.hostname sql = """ - SELECT COALESCE(COUNT(*), 0) FROM events + SELECT COUNT(*) FROM events WHERE type = 'm.room.encrypted' AND sender LIKE ? AND stream_ordering > ? @@ -139,7 +139,7 @@ def _count_messages(txn): async def count_daily_active_e2ee_rooms(self): def _count(txn): sql = """ - SELECT COALESCE(COUNT(DISTINCT room_id), 0) FROM events + SELECT COUNT(DISTINCT room_id) FROM events WHERE type = 'm.room.encrypted' AND stream_ordering > ? """ @@ -161,7 +161,7 @@ async def count_daily_messages(self): def _count_messages(txn): sql = """ - SELECT COALESCE(COUNT(*), 0) FROM events + SELECT COUNT(*) FROM events WHERE type = 'm.room.message' AND stream_ordering > ? """ @@ -178,7 +178,7 @@ def _count_messages(txn): like_clause = "%:" + self.hs.hostname sql = """ - SELECT COALESCE(COUNT(*), 0) FROM events + SELECT COUNT(*) FROM events WHERE type = 'm.room.message' AND sender LIKE ? AND stream_ordering > ? @@ -195,7 +195,7 @@ def _count_messages(txn): async def count_daily_active_rooms(self): def _count(txn): sql = """ - SELECT COALESCE(COUNT(DISTINCT room_id), 0) FROM events + SELECT COUNT(DISTINCT room_id) FROM events WHERE type = 'm.room.message' AND stream_ordering > ? """ @@ -231,7 +231,7 @@ def _count_users(self, txn, time_from): Returns number of users seen in the past time_from period """ sql = """ - SELECT COALESCE(count(*), 0) FROM ( + SELECT COUNT(*) FROM ( SELECT user_id FROM user_ips WHERE last_seen > ? GROUP BY user_id @@ -258,7 +258,7 @@ def _count_r30_users(txn): thirty_days_ago_in_secs = now - thirty_days_in_secs sql = """ - SELECT platform, COALESCE(count(*), 0) FROM ( + SELECT platform, COUNT(*) FROM ( SELECT users.name, platform, users.creation_ts * 1000, MAX(uip.last_seen) @@ -296,7 +296,7 @@ def _count_r30_users(txn): results[row[0]] = row[1] sql = """ - SELECT COALESCE(count(*), 0) FROM ( + SELECT COUNT(*) FROM ( SELECT users.name, users.creation_ts * 1000, MAX(uip.last_seen) FROM users diff --git a/synapse/storage/databases/main/monthly_active_users.py b/synapse/storage/databases/main/monthly_active_users.py index 65b7e307e146..8f09dd8e8751 100644 --- a/synapse/storage/databases/main/monthly_active_users.py +++ b/synapse/storage/databases/main/monthly_active_users.py @@ -59,7 +59,7 @@ async def get_monthly_active_count(self) -> int: def _count_users(txn): # Exclude app service users sql = """ - SELECT COALESCE(count(*), 0) + SELECT COUNT(*) FROM monthly_active_users LEFT JOIN users ON monthly_active_users.user_id=users.name @@ -86,7 +86,7 @@ async def get_monthly_active_count_by_service(self) -> Dict[str, int]: def _count_users_by_service(txn): sql = """ - SELECT COALESCE(appservice_id, 'native'), COALESCE(count(*), 0) + SELECT COALESCE(appservice_id, 'native'), COUNT(*) FROM monthly_active_users LEFT JOIN users ON monthly_active_users.user_id=users.name GROUP BY appservice_id; diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 86c34257168c..29d9d4de9627 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -794,7 +794,7 @@ def _count_daily_user_type(txn): yesterday = int(self._clock.time()) - (60 * 60 * 24) sql = """ - SELECT user_type, COALESCE(count(*), 0) AS count FROM ( + SELECT user_type, COUNT(*) AS count FROM ( SELECT CASE WHEN is_guest=0 AND appservice_id IS NULL THEN 'native' @@ -819,7 +819,7 @@ async def count_nonbridged_users(self): def _count_users(txn): txn.execute( """ - SELECT COALESCE(COUNT(*), 0) FROM users + SELECT COUNT(*) FROM users WHERE appservice_id IS NULL """ ) diff --git a/synapse/storage/databases/main/relations.py b/synapse/storage/databases/main/relations.py index 3368a8b08488..729ff17e2e19 100644 --- a/synapse/storage/databases/main/relations.py +++ b/synapse/storage/databases/main/relations.py @@ -390,7 +390,7 @@ def _get_thread_summary_txn( latest_event_id = row[0] sql = """ - SELECT COALESCE(COUNT(event_id), 0) + SELECT COUNT(event_id) FROM event_relations INNER JOIN events USING (event_id) WHERE diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index 28c4b65bbd4c..6cf6cc8484ba 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -217,7 +217,7 @@ def _count_public_rooms_txn(txn): sql = """ SELECT - COALESCE(COUNT(*), 0) + COUNT(*) FROM ( %(published_sql)s ) published diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py index 9020e0976c7c..a0472e37f5b0 100644 --- a/synapse/storage/databases/main/stats.py +++ b/synapse/storage/databases/main/stats.py @@ -538,7 +538,7 @@ def _fetch_current_state_stats(txn): txn.execute( """ - SELECT COALESCE(count(*), 0) FROM current_state_events + SELECT COUNT(*) FROM current_state_events WHERE room_id = ? """, (room_id,), diff --git a/tests/storage/test_event_federation.py b/tests/storage/test_event_federation.py index c3fcf7e7b405..ecfda7677e0a 100644 --- a/tests/storage/test_event_federation.py +++ b/tests/storage/test_event_federation.py @@ -550,7 +550,7 @@ def test_prune_inbound_federation_queue(self): self.store.db_pool.simple_select_one_onecol( table="federation_inbound_events_staging", keyvalues={"room_id": room_id}, - retcol="COALESCE(COUNT(*), 0)", + retcol="COUNT(*)", desc="test_prune_inbound_federation_queue", ) ) From aaf1ba4905b004a8c201ec4ca80723ea2f5b61a4 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Mon, 13 Dec 2021 19:02:57 +0000 Subject: [PATCH 2/2] Make CI happy --- changelog.d/{11567.misc => 11570.misc} | 0 synapse/storage/databases/main/event_federation.py | 4 +--- 2 files changed, 1 insertion(+), 3 deletions(-) rename changelog.d/{11567.misc => 11570.misc} (100%) diff --git a/changelog.d/11567.misc b/changelog.d/11570.misc similarity index 100% rename from changelog.d/11567.misc rename to changelog.d/11570.misc diff --git a/synapse/storage/databases/main/event_federation.py b/synapse/storage/databases/main/event_federation.py index 0c5ca506d427..bc5ff25d0880 100644 --- a/synapse/storage/databases/main/event_federation.py +++ b/synapse/storage/databases/main/event_federation.py @@ -1485,9 +1485,7 @@ async def _get_stats_for_federation_staging(self): """Update the prometheus metrics for the inbound federation staging area.""" def _get_stats_for_federation_staging_txn(txn): - txn.execute( - "SELECT count(*) FROM federation_inbound_events_staging" - ) + txn.execute("SELECT count(*) FROM federation_inbound_events_staging") (count,) = txn.fetchone() txn.execute(