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

Use Postgres ANY for selecting many values.#6156

Merged
erikjohnston merged 7 commits intodevelopfrom
erikj/postgres_any
Oct 10, 2019
Merged

Use Postgres ANY for selecting many values.#6156
erikjohnston merged 7 commits intodevelopfrom
erikj/postgres_any

Conversation

@erikjohnston
Copy link
Copy Markdown
Member

The advantage of doing col = ANY(?) rather than col IN (?,?,?,...) is mainly to make looking at postgres query stats easier.

@erikjohnston erikjohnston marked this pull request as ready for review October 10, 2019 12:13
@erikjohnston erikjohnston requested a review from a team October 10, 2019 12:13
This means that we can write queries with `col = ANY(?)`, which helps
postgres.
@erikjohnston
Copy link
Copy Markdown
Member Author

FTR I'm not 100% convinced that making the hand written queries more complicated is the right thing to do.

Copy link
Copy Markdown
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm, but a couple of thoughts.

Comment thread synapse/storage/_base.py Outdated
A tuple of SQL query and the args
"""

if isinstance(database_engine, PostgresEngine):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i'd be tempted to make this a method on the database engine, but ymmv

Comment thread synapse/storage/_base.py Outdated
raise


def add_in_list_sql_clause(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not convinced that this function adds much in the way of clarity tbh. I'd be inclined to inline it.

@erikjohnston erikjohnston merged commit 83d8610 into develop Oct 10, 2019
@erikjohnston erikjohnston deleted the erikj/postgres_any branch January 9, 2020 15:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants