txn being redefined in for loop as we are iterating over it

[MadLittleMods]

Spurred on by the note in #10734 about txn being redefined in for loop as we are iterating over it.

We were originally iterating over txn, which can work, but as txn was being used to run a
simple_delete operation inside the loop, it was actually being redefined mid-loop.

#10734

I was curious if there were other cases in the codebase, so I searched for in txn: and found one more case. It looks like this would cause a problem if there was an event where the event_auth_chains -> chain_id was not defined.

synapse/synapse/storage/databases/main/events.py

Lines 578 to 588 in c586d68

	for auth_id, event_type, state_key, chain_id, sequence_number in txn:
	event_to_types[auth_id] = (event_type, state_key)
	if chain_id is None:
	# No chain ID, so the event was persisted out of band.
	# We add to list of events to calculate auth chains for.
	events_to_calc_chain_id_for.add(auth_id)
	event_to_auth_chain[auth_id] = db_pool.simple_select_onecol_txn(
	txn,

Linting

Maybe we should disallow iterating over txn directly in favor of using txn.fetchall()

[erikjohnston]

Oh, good catch. I think its unlikely for that case to ever be hit in practice as its really only used to handle out of band memberships and we likely only have one of those per room anyway. We should still fix it though!

Maybe we should disallow iterating over txn directly in favor of using txn.fetchall()

Using txn can be considerably faster when there's a lot of rows to process, so I'm not super keen on forbidding it entirely. Ideally mypy would have a way to annotate that we can't reuse txn here, but I don't think its possible.