This repository was archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Couldn't load subscription status.
- Fork 2.1k
simple_* methods handle None incorrectly for key valuesΒ #14157
Copy link
Copy link
Open
Labels
A-DatabaseDB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the dbDB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the dbO-UncommonMost users are unlikely to come across this or unexpected workflowMost users are unlikely to come across this or unexpected workflowS-MinorBlocks non-critical functionality, workarounds exist.Blocks non-critical functionality, workarounds exist.T-DefectBugs, crashes, hangs, security vulnerabilities, or other reported issues.Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Description
Generally the simple_* methods do not handle None properly when used as a key-value (the exact arguments differ by method). See #14138 for real fallout from this.
simple_upsert_emulated_txn has special handling for null which should likely be abstracted:
synapse/synapse/storage/database.py
Lines 1304 to 1310 in b4ec4f5
| def _getwhere(key: str) -> str: | |
| # If the value we're passing in is None (aka NULL), we need to use | |
| # IS, not =, as NULL = NULL equals NULL (False). | |
| if keyvalues[key] is None: | |
| return "%s IS ?" % (key,) | |
| else: | |
| return "%s = ?" % (key,) |
This is mostly a footgun that we could avoid by either:
- Raising an exception (or asserting) when a key-value is
None. - Automatically handling
Noneand turning it intoIS NONE(which is whatsimple_upsert_emulated_txndoes).
MadLittleMods
Metadata
Metadata
Assignees
Labels
A-DatabaseDB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the dbDB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the dbO-UncommonMost users are unlikely to come across this or unexpected workflowMost users are unlikely to come across this or unexpected workflowS-MinorBlocks non-critical functionality, workarounds exist.Blocks non-critical functionality, workarounds exist.T-DefectBugs, crashes, hangs, security vulnerabilities, or other reported issues.Bugs, crashes, hangs, security vulnerabilities, or other reported issues.