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

Support explicitly naming constraints #100

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

andrewclink
Copy link

This PR allows one to pass an explicit Postgres constraint name to upsert_keys:

class MyClass < ActiveRecord::Base
    upsert_keys constraint: 'my_constraint'
end

This generates SQL like

INSERT INTO ... ON CONFLICT ON CONSTRAINT my_constraint DO UPDATE...

My only question is the proper quoting of the constraint name. On one hand, I wouldn't expect a dev to SQL-inject themselves. An attacker already having access to ActiveRecord::Base class methods probably obviates the need to be concerned with security. However, I'm raising it now if anyone else thinks it needs to be addressed.

Tests included (which requires migrating your test database). All are passing ✅

@olleolleolle
Copy link
Collaborator

The CI complains in a step of the migrations on Rails 5.0, 5.1:

ArgumentError: Unknown migration version "5.2"; expected one of "4.2", "5.0", "5.1"

@andrewclink
Copy link
Author

Sorry, that was an auto-generated migration file that slipped through.

@olleolleolle
Copy link
Collaborator

@andrewclink Perhaps all that remains is a rebase (to get rid of conflicts)?

@olleolleolle
Copy link
Collaborator

@andrewclink Perhaps you can rebase and resolve conflicts?

@andrewclink
Copy link
Author

Sure, I actually have a few free hours today! Let me take a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants