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

Fix sqlite syntax for upserts. #14171

Merged
merged 2 commits into from
Oct 13, 2022
Merged

Fix sqlite syntax for upserts. #14171

merged 2 commits into from
Oct 13, 2022

Conversation

clokep
Copy link
Member

@clokep clokep commented Oct 13, 2022

Fixes the backfill script given in #13394. I missed parentheses for sqlite.

@clokep clokep marked this pull request as ready for review October 13, 2022 16:07
@clokep clokep requested a review from a team as a code owner October 13, 2022 16:07
Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

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

(Any reason for not using one of the upsert helpers here?)

@clokep
Copy link
Member Author

clokep commented Oct 13, 2022

(Any reason for not using one of the upsert helpers here?)

They can't be used: they don't support providing both keys and values, but doing nothing on conflict. (They assume that you want to apply the new values on conflict.)

@clokep clokep merged commit 2019b60 into develop Oct 13, 2022
@clokep clokep deleted the clokep/fix-bg-update branch October 13, 2022 16:53
@DMRobertson
Copy link
Contributor

(Any reason for not using one of the upsert helpers here?)

They can't be used: they don't support providing both keys and values, but doing nothing on conflict. (They assume that you want to apply the new values on conflict.)

Are you sure? This comment (err, which I wrote) suggests otherwise:

If (values) is empty, the resulting query is slighlty simpler:
INSERT INTO table VALUES (insertion_values)
ON CONFLICT (keyvalues)
DO NOTHING; -- do not overwrite any columns

Will gladly amend the comment if it is wrong.

@clokep
Copy link
Member Author

clokep commented Oct 13, 2022

Are you sure? This comment (err, which I wrote) suggests otherwise:

The comment is correct: in this case we do want to pass values, but we don't want to use them to update, only to insert.

I'm fairly certain this isn't possible with the current API since if you pass values it tries to update on conflict.

@DMRobertson
Copy link
Contributor

Note that there are three dictionaries by which you can provide column/value pairs: keybalues, values and insertion_values. I might be missing something, but I think

simple_upsert(
    table="threads",
    keyvalues={"room_id": ..., "thread_id": ... },
    values={},
    insertion_values={"topological_ordering": ..., "stream_ordering": ..., "latest_event_id": ...},
)

would work?

(I'm not saying we should write it this way. If anything, I'm advocating for the opposite: the manually written sql is probably clearer, and the fact that we're not in agreement about the helper suggests that its behaviour is confusing.)

@clokep
Copy link
Member Author

clokep commented Oct 13, 2022

Note that there are three dictionaries by which you can provide column/value pairs: keybalues, values and insertion_values. I might be missing something, but I think

This would need to use simple_upsert_many which does not offer this functionality:

async def simple_upsert_many(
self,
table: str,
key_names: Collection[str],
key_values: Collection[Collection[Any]],
value_names: Collection[str],
value_values: Collection[Collection[Any]],
desc: str,
lock: bool = True,
) -> None:

@DMRobertson
Copy link
Contributor

Ahh thanks, I'd missed that you were upserting multiple rows in one query.

clokep added a commit that referenced this pull request Oct 14, 2022
@clokep
Copy link
Member Author

clokep commented Oct 14, 2022

I updated the changelog of this in 5a983cc to match the changes done in #14175.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 29, 2022
Upstream changes:

Synapse 1.70.1 (2022-10-28)
===========================

(bugfixes)


Synapse 1.70.0 (2022-10-26)
===========================

Features
--------

- Support for
  [MSC3856](matrix-org/matrix-spec-proposals#3856):
  threads list
  API. ([\#13394](matrix-org/synapse#13394),
  [\#14171](matrix-org/synapse#14171),
  [\#14175](matrix-org/synapse#14175))

- Support for thread-specific notifications & receipts
  ([MSC3771](matrix-org/matrix-spec-proposals#3771)
  and
  [MSC3773](matrix-org/matrix-spec-proposals#3773)). ([\#13776](matrix-org/synapse#13776),
  [\#13824](matrix-org/synapse#13824),
  [\#13877](matrix-org/synapse#13877),
  [\#13878](matrix-org/synapse#13878),
  [\#14050](matrix-org/synapse#14050),
  [\#14140](matrix-org/synapse#14140),
  [\#14159](matrix-org/synapse#14159),
  [\#14163](matrix-org/synapse#14163),
  [\#14174](matrix-org/synapse#14174),
  [\#14222](matrix-org/synapse#14222))

- Stop fetching missing `prev_events` after we already know their
  signature is
  invalid. ([\#13816](matrix-org/synapse#13816))

- Send application service access tokens as a header (and query
  parameter). Implements
  [MSC2832](matrix-org/matrix-spec-proposals#2832). ([\#13996](matrix-org/synapse#13996))

- Ignore server ACL changes when generating pushes. Implements
  [MSC3786](matrix-org/matrix-spec-proposals#3786). ([\#13997](matrix-org/synapse#13997))

- Experimental support for redirecting to an implementation of a
  [MSC3886](matrix-org/matrix-spec-proposals#3886)
  HTTP rendezvous
  service. ([\#14018](matrix-org/synapse#14018))

- The `/relations` endpoint can now be used on
  workers. ([\#14028](matrix-org/synapse#14028))

- Advertise support for Matrix 1.3 and 1.4 on
  `/_matrix/client/versions`. ([\#14032](matrix-org/synapse#14032),
  [\#14184](matrix-org/synapse#14184))

- Improve validation of request bodies for the [Device
  Management](https://spec.matrix.org/v1.4/client-server-api/#device-management)
  and [MSC2697 Device
  Dehyrdation](matrix-org/matrix-spec-proposals#2697)
  client-server API
  endpoints. ([\#14054](matrix-org/synapse#14054))

- Experimental support for
  [MSC3874](matrix-org/matrix-spec-proposals#3874):
  Filtering threads from the `/messages`
  endpoint. ([\#14148](matrix-org/synapse#14148))

- Improve the validation of the following PUT endpoints:
  [`/directory/room/{roomAlias}`](https://spec.matrix.org/v1.4/client-server-api/#put_matrixclientv3directoryroomroomalias),
  [`/directory/list/room/{roomId}`](https://spec.matrix.org/v1.4/client-server-api/#put_matrixclientv3directorylistroomroomid)
  and
  [`/directory/list/appservice/{networkId}/{roomId}`](https://spec.matrix.org/v1.4/application-service-api/#put_matrixclientv3directorylistappservicenetworkidroomid). ([\#14179](matrix-org/synapse#14179))


Deprecations and Removals
-------------------------

- Remove the experimental implementation of
  [MSC3772](matrix-org/matrix-spec-proposals#3772). ([\#14094](matrix-org/synapse#14094))

- Remove the unstable identifier for
  [MSC3715](matrix-org/matrix-spec-proposals#3715). ([\#14106](matrix-org/synapse#14106),
  [\#14146](matrix-org/synapse#14146))
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