Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Rewrite get push actions queries #13597

Merged
merged 6 commits into from
Aug 24, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 50 additions & 2 deletions synapse/storage/databases/main/event_push_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,26 @@ async def get_unread_push_actions_for_user_in_range_for_http(
),
)

def get_push_actions(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to be customary, this name should include the _txn suffix because it takes a Transaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

txn: LoggingTransaction,
) -> List[Tuple[str, str, int, str, bool]]:
sql = """
SELECT ep.event_id, ep.room_id, ep.stream_ordering, ep.actions, ep.highlight
FROM event_push_actions AS ep
WHERE
ep.user_id = ?
AND ep.stream_ordering > ?
AND ep.stream_ordering <= ?
AND ep.notif = 1
ORDER BY ep.stream_ordering ASC LIMIT ?
"""
txn.execute(sql, (user_id, min_stream_ordering, max_stream_ordering, limit))
return cast(List[Tuple[str, str, int, str, bool]], txn.fetchall())

push_actions = await self.db_pool.runInteraction(
"get_unread_push_actions_for_user_in_range_http", get_push_actions
)

# find rooms that have a read receipt in them and return the next
# push actions
def get_after_receipt(
Expand Down Expand Up @@ -615,7 +635,10 @@ def get_no_receipt(
stream_ordering=row[2],
actions=_deserialize_action(row[3], row[4]),
)
for row in after_read_receipt + no_read_receipt
for row in push_actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to help with readability, especially since you're adding a condition, I'd be tempted to destructure this tuple and then refer to everything by name:

Suggested change
for row in push_actions
for event_id, room_id, stream_ordering, actions, highlight in push_actions

Copy link
Contributor Author

@Fizzadar Fizzadar Aug 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much cleaner! b7a66e3

# Only include push actions with a stream ordering after any receipt, or without any
# receipt present (invited to but never read rooms).
if row[2] > receipts_by_room.get(row[1], 0)
]

# Now sort it so it's ordered correctly, since currently it will
Expand Down Expand Up @@ -659,6 +682,28 @@ async def get_unread_push_actions_for_user_in_range_for_email(
),
)

def get_push_actions(
txn: LoggingTransaction,
) -> List[Tuple[str, str, int, str, bool, int]]:
sql = """
SELECT ep.event_id, ep.room_id, ep.stream_ordering, ep.actions,
ep.highlight, e.received_ts
FROM event_push_actions AS ep
INNER JOIN events AS e USING (room_id, event_id)
WHERE
ep.user_id = ?
AND ep.stream_ordering > ?
AND ep.stream_ordering <= ?
AND ep.notif = 1
ORDER BY ep.stream_ordering DESC LIMIT ?
"""
txn.execute(sql, (user_id, min_stream_ordering, max_stream_ordering, limit))
return cast(List[Tuple[str, str, int, str, bool, int]], txn.fetchall())

push_actions = await self.db_pool.runInteraction(
"get_unread_push_actions_for_user_in_range_email", get_push_actions
)

# find rooms that have a read receipt in them and return the most recent
# push actions
def get_after_receipt(
Expand Down Expand Up @@ -758,7 +803,10 @@ def get_no_receipt(
actions=_deserialize_action(row[3], row[4]),
received_ts=row[5],
)
for row in after_read_receipt + no_read_receipt
for row in push_actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto for destructuring here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Only include push actions with a stream ordering after any receipt, or without any
# receipt present (invited to but never read rooms).
if row[2] > receipts_by_room.get(row[1], 0)
]

# Now sort it so it's ordered correctly, since currently it will
Expand Down