Support providing an index predicate for upserts.

[clokep]

This is required for #13782, but I'm pulling it out into a separate pull request. You can see this in-use:

synapse/synapse/storage/databases/main/receipts.py

Lines 707 to 732 in f9f3f77

	keyvalues = {
	"room_id": room_id,
	"receipt_type": receipt_type,
	"user_id": user_id,
	}
	where_clause = ""
	if thread_id is None:
	where_clause = "thread_id IS NULL"
	else:
	keyvalues["thread_id"] = thread_id
	self.db_pool.simple_upsert_txn(
	txn,
	table="receipts_linearized",
	keyvalues=keyvalues,
	values={
	"stream_id": stream_id,
	"event_id": event_id,
	"event_stream_ordering": stream_ordering,
	"data": json_encoder.encode(data),
	},
	where_clause=where_clause,
	# receipts_linearized has a unique constraint on
	# (user_id, room_id, receipt_type), so no need to lock
	lock=False,
	)

I decided to just take a raw where_clause (as a string) instead of key-value pairs to match register_background_index_update (and to simplify the implementation).

[DMRobertson]

Seems reasonable to me.

I wasn't fully sure what you meant by "index predicate". Quoting https://www.postgresql.org/docs/current/sql-insert.html:

Used to allow inference of partial unique indexes. Any indexes that satisfy the predicate (which need not actually be partial indexes) can be inferred. Follows CREATE INDEX format. SELECT privilege on any column appearing within index_predicate is required.

Being honest I don't fully understand what that means. But from the POV of this change it's clearly something that lets you fill in the WHERE clause in a sensible way.

[DMRobertson]

I'm slightly anxious that accepting a freeform string as where_clause might cause us to accidentally expose an injection site in the future. #13473 would help here.

[clokep]

Seems reasonable to me.

I wasn't fully sure what you meant by "index predicate". Quoting postgresql.org/docs/current/sql-insert.html:

Used to allow inference of partial unique indexes. Any indexes that satisfy the predicate (which need not actually be partial indexes) can be inferred. Follows CREATE INDEX format. SELECT privilege on any column appearing within index_predicate is required.

Being honest I don't fully understand what that means. But from the POV of this change it's clearly something that lets you fill in the WHERE clause in a sensible way.

I found https://miguoliang.com/posts/postgresql-make-unique-index-support-null-column-by-creating-partial-index/ useful in understanding how to use this and why it is useful.

[clokep]

I'm slightly anxious that accepting a freeform string as where_clause might cause us to accidentally expose an injection site in the future. #13473 would help here.

I'm not sure how to mitigate this at the moment. 😢

[DMRobertson]

I'm slightly anxious that accepting a freeform string as where_clause might cause us to accidentally expose an injection site in the future. #13473 would help here.

I'm not sure how to mitigate this at the moment. cry

Careful review, I think; but it doesn't seem like there's a good systematic option to do so right now. Proceed cautiously I guess?

[DMRobertson]

I found miguoliang.com/posts/postgresql-make-unique-index-support-null-column-by-creating-partial-index useful in understanding how to use this and why it is useful.

Thanks, this was very instructive. My understanding:

• in order for upsert to work you need to detect when you are "changing" something (as opposed to inserting a second, unrelated copy of something)
• we do this conceptually by having a unique constraint on some column(s) and detecting races/conflicts
• in practice this is implemented by having a unique index on those columns
• sometimes we don't have a unique index on (C1, C2, C3, ...); we only have a unique index on a subset (C1, C2, C3, ...) WHERE stuff
• "stuff" is formally called an "index predicate"
• we can use these sorts of partial/limited unique indices if they exist to detect conflicts on insertion if we want to.

[DMRobertson]

No objections other than my wording suggestion. Thanks!