Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow upserting by exclusion constraints and index expressions #2123

Open
wattroll opened this issue Jan 10, 2022 · 6 comments
Open

Allow upserting by exclusion constraints and index expressions #2123

wattroll opened this issue Jan 10, 2022 · 6 comments
Labels
idea Needs of discussion to become an enhancement, not ready for implementation

Comments

@wattroll
Copy link

Current query syntax allows on_conflict with a set of fields that must have a matching UNIQUE constraint to upsert based on them.

ON CONFLICT can also be used with explicit ON CONSTRAINT <constraint-name> directive. This is very handy as it allows upserting according to exclusion constraints. This allows upsert based on almost arbitrary business rules when the schema allows it (by defining a constraint).

Current situation is a bit limiting. It is possible to define a resource which rejects certain inserts, but not possible to handle those rejections gracefully.

Will there be some pitfalls if support for this was added?

What would be a good syntax for this feature?

@wattroll wattroll changed the title Allow upserting by an exclusion constraints Allow upserting by exclusion constraints Jan 10, 2022
@wolfgangwalther wolfgangwalther added the idea Needs of discussion to become an enhancement, not ready for implementation label Jan 10, 2022
@wolfgangwalther
Copy link
Member

I don't have an idea about syntax, yet - but I like the idea. Even though we were noting somewhere in #2070 (long thread), that constraint names basically violate our schema isolation approach, I still think we can't work around those in general, especially not for embedding. So it seems consistent to follow up on that and allow to use them for conflict resolution, too.

@steve-chavez
Copy link
Member

Hm, the postgresql docs mention:

It is often preferable to use unique index inference rather than naming a constraint directly using ON CONFLICT ON CONSTRAINT constraint_name. Inference will continue to work correctly when the underlying index is replaced by another more or less equivalent index in an overlapping way, for example when using CREATE UNIQUE INDEX ... CONCURRENTLY before dropping the index being replaced.

conflict_target can perform unique index inference. When performing inference, it consists of one or more index_column_name columns and/or index_expression expressions, and an optional index_predicate.

https://www.postgresql.org/docs/current/sql-insert.html#SQL-ON-CONFLICT

@wattroll So wouldn't using ON CONFLICT with a list columns be preferrable to ON CONSTRAINT? Are there cases which cannot be resolved in that way?

@wolfgangwalther
Copy link
Member

While following https://www.postgresql.org/message-id/flat/5b238fdd-16a2-4b63-a347-c63813ebccb0%40www.fastmail.com I thought about why using constraint names in general could be a good idea. In a way a constraint name hides the implementation details of which columns are used. When used in JOIN / embedding, we don't even need to know which columns are part of the FK, we can just give this a meaningful name and abstract the implementation away from the user joining tables. Fewer queries to change, fewer mistakes to make... I think it's useful.

I think that principle still applies here. The tip given in the docs is not very useful imho. If I had to change a unique index, which I had given a meaningful name (and I do that all the time, because of error reporting when inserting conflicting rows), then I would do the following:

  • create new index with different name CREATE UNIQUE INDEX ... CONCURRENTLY
  • once that's done, rename both indexes in a single transaction to make the new index have the meaningful name
  • then drop the old index

This would not break any queries that use the constraint name - in fact it would enable them to work correctly, even when renaming columns.

@wattroll
Copy link
Author

@wolfgangwalther I see the concern regarding the schema isolation. As you say, it can be seen from several perspectives. On one side sending the constraint name seems like relying on internal details of a schema. On the other hand, specifying columns to be unique-by is not much different (in terms of schema isolation), it makes an expectation that those columns would be there with those exact names and there will be a unique constraint on them of some kind. So explicitly naming a constraint can also be useful in that regard.

Is there a way for us to see if the constraint has a default auto-generated name or the database administrator did provide something better? Then we could maybe get the best of both worlds by not allowing to specify a constraint name UNLESS it has been named explicitly.

@steve-chavez Indeed, if it's possible to achieve what's needed by only specifying column names it's a better way as it would allow postgresql to figure out what is the best index for that query. I'll make an example out of my use-case, so that we have something to work with (and include in the docs if we go forward with this).

@wolfgangwalther
Copy link
Member

On one side sending the constraint name seems like relying on internal details of a schema. On the other hand, specifying columns to be unique-by is not much different (in terms of schema isolation),

It is quite different in general, because the column names can be from the exposed view schema. Schema isolation is about hiding details from the hidden table schema...

it makes an expectation that those columns would be there with those exact names and there will be a unique constraint on them of some kind.

... although I don't really know which column names are used for the on_conflict, when dealing with updateable views. Using ON CONFLICT with updatable views is quite limited anyway. Afaik, it only works with auto-updateable views. See also https://www.postgresql.org/message-id/flat/ddd29182-86fb-4c2b-a930-85dcfe2272d0%40www.fastmail.com.

@steve-chavez
Copy link
Member

There's also a use case for supporting index expressions in ON CONFLICT: https://github.com/orgs/supabase/discussions/14898#discussioncomment-6261112

The on_conflict parameter doesn't support expressions. Not sure yet how we would do it.

@steve-chavez steve-chavez changed the title Allow upserting by exclusion constraints Allow upserting by exclusion constraints and index expressions Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea Needs of discussion to become an enhancement, not ready for implementation
Development

No branches or pull requests

3 participants