-
Notifications
You must be signed in to change notification settings - Fork 88
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
Bulk import #168
Bulk import #168
Changes from all commits
814de94
d1b2134
93b6e67
6435d63
85c36a0
73e7de5
1a03145
54dd5b7
c74d126
e0e1be4
84364d9
2d15769
bbd2f7f
2425ad2
87009d5
acc4a6e
4b10e27
87268a0
f6dcf2a
e3e525a
371d0b4
9fe4bcb
5b31b80
027be38
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
require "../../spec_helper" | ||
|
||
{% for adapter in GraniteExample::ADAPTERS %} | ||
module {{adapter.capitalize.id}} | ||
describe "{{ adapter.id }} .import" do | ||
it "should import 3 new objects" do | ||
to_import = [ | ||
Parent.new(name: "ImportParent1"), | ||
Parent.new(name: "ImportParent2"), | ||
Parent.new(name: "ImportParent3"), | ||
] | ||
Parent.import(to_import) | ||
Parent.all("WHERE name LIKE ?", ["Import%"]).size.should eq 3 | ||
end | ||
|
||
it "should work with on_duplicate_key_update" do | ||
to_import = [ | ||
Parent.new(id: 111, name: "ImportParent1"), | ||
Parent.new(id: 112, name: "ImportParent2"), | ||
Parent.new(id: 113, name: "ImportParent3"), | ||
] | ||
|
||
Parent.import(to_import) | ||
|
||
to_import = [ | ||
Parent.new(id: 112, name: "ImportParent112"), | ||
] | ||
|
||
Parent.import(to_import, update_on_duplicate: true, columns: ["name"]) | ||
|
||
if parent = Parent.find 112 | ||
parent.name.should be "ImportParent112" | ||
parent.id.should eq 112 | ||
end | ||
end | ||
|
||
it "should work with on_duplicate_key_ignore" do | ||
to_import = [ | ||
Parent.new(id: 111, name: "ImportParent1"), | ||
Parent.new(id: 112, name: "ImportParent2"), | ||
Parent.new(id: 113, name: "ImportParent3"), | ||
] | ||
|
||
Parent.import(to_import) | ||
|
||
to_import = [ | ||
Parent.new(id: 113, name: "ImportParent113"), | ||
] | ||
|
||
Parent.import(to_import, ignore_on_duplicate: true) | ||
|
||
if parent = Parent.find 113 | ||
parent.name.should be "ImportParent3" | ||
parent.id.should eq 113 | ||
end | ||
end | ||
end | ||
end | ||
{% end %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,9 +7,36 @@ module Granite::ORM::Transactions | |
@updated_at : Time? | ||
@created_at : Time? | ||
|
||
# The import class method will run a batch INSERT statement for each model in the array | ||
# the array must contain only one model class | ||
# invalid model records will be skipped | ||
def self.import(model_array : Array(self)) | ||
begin | ||
@@adapter.import(table_name, primary_name, fields.dup, model_array) | ||
rescue err | ||
raise DB::Error.new(err.message) | ||
end | ||
end | ||
|
||
def self.import(model_array : Array(self), update_on_duplicate : Bool, columns : Array(String)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since you would never use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would there be a nice way to also require columns key when using the update option? |
||
begin | ||
@@adapter.import(table_name, primary_name, fields.dup, model_array, update_on_duplicate: update_on_duplicate, columns: columns) | ||
rescue err | ||
raise DB::Error.new(err.message) | ||
end | ||
end | ||
|
||
def self.import(model_array : Array(self), ignore_on_duplicate : Bool) | ||
begin | ||
@@adapter.import(table_name, primary_name, fields.dup, model_array, ignore_on_duplicate: ignore_on_duplicate) | ||
rescue err | ||
raise DB::Error.new(err.message) | ||
end | ||
end | ||
|
||
# The save method will check to see if the primary exists yet. If it does it | ||
# will call the update method, otherwise it will call the create method. | ||
# This will update the timestamps apropriately. | ||
# This will update the timestamps appropriately. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🥇 |
||
def save | ||
return false unless valid? | ||
|
||
|
@@ -91,7 +118,7 @@ module Granite::ORM::Transactions | |
end | ||
end | ||
|
||
module ClassMethods | ||
module ClassMethods | ||
def create(**args) | ||
create(args.to_h) | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
your
**options
can be a little cleaner and have a similar effect:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would mean i would need to have a defined param for each options right?
As of now there is
on_duplicate_key_ignore: Bool
,on_duplicate_key_update: Array(Strings) to be updated
, and possibly in future alsovalidate: true
, as giving option to ignore validation might speed up large imports. I don't have strong feelings one way or another.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can probably have the best of both worlds using a crystal overloaded method signature. If you need, here are the docs on overloading
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t have strong opinions on this either. I just noticed a rubyism that might be able to be cleaned up with some crystal.