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

Bulk import #168

Merged
merged 24 commits into from
Apr 26, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
814de94
Start of bulk-import method
Blacksmoke16 Mar 30, 2018
d1b2134
Use parameterized statement
Blacksmoke16 Mar 30, 2018
93b6e67
Handle duplicate key IGNORE/UPDATE
Blacksmoke16 Mar 31, 2018
6435d63
Add commas between update values
Blacksmoke16 Mar 31, 2018
85c36a0
merge in the fields unity code
Blacksmoke16 Mar 31, 2018
73e7de5
Remove now undeeded primary_auto variable
Blacksmoke16 Mar 31, 2018
1a03145
Add import for pg adapter
Blacksmoke16 Mar 31, 2018
54dd5b7
Also quote other column key
Blacksmoke16 Mar 31, 2018
c74d126
Add adapter for sqlite
Blacksmoke16 Mar 31, 2018
e0e1be4
Merge branch 'master' into bulk-import
faustinoaq Apr 4, 2018
84364d9
Fix spec
Blacksmoke16 Apr 7, 2018
2d15769
Add more tests, fix sqlite adapter
Blacksmoke16 Apr 7, 2018
bbd2f7f
Update readme
Blacksmoke16 Apr 7, 2018
2425ad2
Fix spacing issue in sqlite adapater
Blacksmoke16 Apr 7, 2018
87009d5
Fix indentation
Blacksmoke16 Apr 7, 2018
acc4a6e
Fix typos in readme examples
Blacksmoke16 Apr 7, 2018
4b10e27
Merge branch 'master' into bulk-import
faustinoaq Apr 7, 2018
87268a0
Fix indentations
Blacksmoke16 Apr 7, 2018
f6dcf2a
Add overload methods for update/ignore_on_duplicate, update readme
Blacksmoke16 Apr 8, 2018
e3e525a
Add = sign to model arrays in readme
Blacksmoke16 Apr 12, 2018
371d0b4
Merge branch 'master' into bulk-import
drujensen Apr 22, 2018
9fe4bcb
Specify imports do not trigger callbacks yet
Blacksmoke16 Apr 25, 2018
5b31b80
proper grammar
Blacksmoke16 Apr 25, 2018
027be38
Merge branch 'master' into bulk-import
drujensen Apr 26, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
/.crystal/
/doc/
/config/*.db
/.idea/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this is an editor thing. You should setup and ignore your editor files in a system wide global gitignore file instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Blacksmoke16 interesting, Are you coding with Intellij Idea? just curious, What plugin are you using to have crystal support? 😅

Copy link
Contributor Author

@Blacksmoke16 Blacksmoke16 Mar 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@faustinoaq I am yes :) Using Rubymine with https://github.com/intellij-crystal/intellij-crystal

Is pretty meh in regards to full native support like Ruby in Rubymine, but it provides syntax highlighting and that's good enough for me atm.


# Libraries don't need dependency lock
# Dependencies will be locked in application that uses them
Expand Down
3 changes: 3 additions & 0 deletions src/adapter/base.cr
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ abstract class Granite::Adapter::Base
# This will insert a row in the database and return the id generated.
abstract def insert(table_name, fields, params, lastval) : Int64

# This will insert an array of models as one insert statement
abstract def import(table_name, primary_name, primary_auto, fields, model_array)

# This will update a row in the database.
abstract def update(table_name, primary_name, fields, params)

Expand Down
25 changes: 25 additions & 0 deletions src/adapter/mysql.cr
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,31 @@ class Granite::Adapter::Mysql < Granite::Adapter::Base
end
end

def import(table_name, primary_name, primary_auto, fields, model_array)
statement = String.build do |stmt|
stmt << "INSERT INTO #{quote(table_name)} ("
stmt << "#{quote(primary_name)}, " unless primary_auto
stmt << fields.map { |field| quote(field) }.join(", ")
stmt << ") VALUES "

model_array.each do |model|
if model.responds_to? :to_h
next unless model.valid?
stmt << "("
stmt << model.to_h[primary_name].to_s + ", " unless primary_auto
stmt << fields.map { |field| model.to_h[field].is_a?(String) ? "'" + model.to_h[field].to_s + "'" : model.to_h[field] }.join(", ")
Copy link
Contributor Author

@Blacksmoke16 Blacksmoke16 Mar 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of how I'm handling the quoting of String type fields. Any better ideas?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The adapters now provide a quote method but the functionality is has some bugs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The quoting method I do use for column/table names. However, since it quotes with the backtick that couldn't be used for string values afaik?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't see that you are quoting values. A parameterized query is better for that, and I believe the adapters automatically do what they need to with values in parameterized queries.

Copy link
Contributor Author

@Blacksmoke16 Blacksmoke16 Mar 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my initial thought, however when I tried it I kept getting like db params invalid format or something like that (not home atm to look). The params should just be an array of values, which the first value of array maps to first ? placeholder correct?

I'll mess with it again and see if I can get the params working prob this weekend.

stmt << "),"
end
end
end.chomp(",")

log statement

open do |db|
db.exec statement
end
end

private def last_val
return "SELECT LAST_INSERT_ID()"
end
Expand Down
14 changes: 13 additions & 1 deletion src/granite_orm/transactions.cr
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,21 @@ 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)
raise ArgumentError.new("Model class mismatch: expected array of only #{self} models.") unless model_array.all? { |model| model.class == self }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this definition be changed so that the compiler will enforce the type constraint instead of a runtime error?

First guess, something like this might work:

def self.import(model_array : Array(self))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooo yes! I will do that tonight (still getting used to static typed land :P)

begin
@@adapter.import(table_name, primary_name, {{primary_auto}}, fields, model_array)
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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

def save
return false unless valid?

Expand Down