Closed abinoda closed 3 years ago
@jesjos Let me know if this makes sense
@jesjos I'm using this in https://pullreminders.com to help with all the queries related to sync data with GitHub. It's working great overall.
I am having the very same issue. I'd like to do "ON CONFLICT DO NOTHING". There seems to be some support for this: https://github.com/jesjos/active_record_upsert/search?utf8=%E2%9C%93&q=nothing&type=
But I don't know how to use it.
@jjb @jesjos I'm not sure it should be a DO NOTHING
- it should probably just return the results from the existing/conflicting record in the db?
@jesjos Any thoughts on this issue?
So the purpose is basically to ensure that a minimal record exists in the database?
@jesjos I feel like the purpose is no different than any other upsert.
This is what it looks like to do a find or create....
user_car = UserCar.find_by(user_id: 1, car_id: 2) ||
UserCar.create!(
user_id: 1,
car_id: 2
)
In addition to being verbose, this requires two separate roundtrips to the database. It would be great to be able to replace this with:
UserCar.upsert!(
user_id: 1,
car_id: 2
)
Right now running this ^ code blows up because it tries to execute a malformed ON CONFLICT UPDATE
clause. Instead it should execute a ON CONFLICT RETURN
clause which just returns the row without attempting an update.
I ran in to this issue as well, but noticed that the error doesn't happen on master.
I've traced it back to the removal of the map(&:to_s)
in PR #70. (Though I'm not entirely sure why)
It's still executing ON CONFLICT DO UPDATE
with the values passed in the upsert
(so not a DO NOTHING
) - but properly returns an object.
SQL looks like this on master:
INSERT INTO "user_cars" ("user_id", "car_id") VALUES (123, 456 ) ON CONFLICT ("user_id","car_id") DO UPDATE SET "user_id" = 123, "car_id" = 456 RETURNING *
Is there a plan for the next release?
@ashton-mccrate Thanks for the test and for reminding me to release. I created a v0.9.1
.
@abinoda Does the issue persist with what's released now?
@olleolleolle Sorry, I missed your comment for some reason. I'll test this out soon.
@olleolleolle I am still getting this error on active_record_upsert-0.9.4
.
Here is my simple test case:
# Schema
create_table :cats do |t|
t.string :name
end
add_index :cats, :name, unique: true
# Model
class Cat < ActiveRecord::Base
upsert_keys [:name]
end
# Test
Cat.upsert(name: "nyan")
With a result:
PG::SyntaxError: ERROR: syntax error at or near "RETURNING"
LINE 1: ...e") VALUES ($1) ON CONFLICT ("name") DO UPDATE RETURNING ...
^
: INSERT INTO "cats" ("name") VALUES ($1) ON CONFLICT ("name") DO UPDATE RETURNING *, (xmax = 0) AS _upsert_created_record
I believe this is because the persistence code removes the upsert_key
from the attributes to set.:
upsert_attributes_names = upsert_attributes_names - [*upsert_keys, 'created_at']
...
values_for_upsert = existing_attributes.select { |(name, _value)| upsert_attributes_names.include?(name) }
insert_manager = arel_table.compile_upsert(
upsert_keys,
upsert_options,
_substitute_values(values_for_upsert),
_substitute_values(existing_attributes),
wheres
)
But I have no idea why that's necessary and it appears to have been around for quite awhile.
Sorry, that was not quite right. I figured out the problem:
If I use this model (keys as strings), I get the error:
class Cat < ActiveRecord::Base
upsert_keys ["name"]
end
But with this model (keys as symbols) it works fine:
class Cat < ActiveRecord::Base
upsert_keys [:name]
end
I would say the reported bug is resolved. (Supporting string instead of symbol for a column name would be a separate feature request, but I'm happy to stick to symbols.)
I appreciate the analysis, and will close this issue.
If I have a table like this:
And a model like:
And then run:
It throws an error due to trying to run a bad query. This seems to be happening because it tries to execute an
ON CONFLICT UPDATE
clause but there's nothing to update so it just leaves it blank, resulting in an error. Instead it should runON CONFLICT RETURN