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 explicit insert of primary key field #50

Open
spalladino opened this issue May 4, 2016 · 3 comments
Open

Support explicit insert of primary key field #50

spalladino opened this issue May 4, 2016 · 3 comments

Comments

@spalladino
Copy link

I'd need to explicitly insert the ID field when creating a new record, since I'm not using autoincremental IDs. Following up the changes needed in the postgres adapter, they seem quite simple: just removing a few conditions regarding primary_field in lines 46, 46 and 128.

Do you think that supporting this case would be useful? If so, how do you think it could be specified that in the model DSL? Something like primary explicit id : Int? If you think it's worthwhile, I can try to set something up and send you a PR.

@waterlink
Copy link
Owner

Hi @spalladino

What is the current behavior, when someone operates on the table without auto-increment and supplies id explicitly? My assumption, that it should just work, at least for postgres adapter, not sure about mysql and sqlite adapters though.

@spalladino
Copy link
Author

Currently the adapter deletes the ID field from the fields to insert, and the insert fails due to a non-null constraint on the PK. See here where the primary_field is removed from the list of fields in the INSERT clause. Also, the number of field_refs ($1, $2, etc) generated is equal to the number of fields, minus one (the PK field).

@waterlink
Copy link
Owner

Here update functionality removes primary_field too. I believe that is okay.

So it looks like the difference in the behavior have to be on the adapter's side (create(fields) function).

Maybe second overload abstract def create(id, fields) for the adapter protocol would be good here, which would indicate creation with explicit id value. And on the side of the active_record shard it should be used when id is specified explicitly.

I like the idea of primary explicit id : Int. (probably simpler to have explicit_primary id : Int)

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

No branches or pull requests

2 participants