From 814de94b1b5b754efea4f3c09604be7acf153e1d Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Thu, 29 Mar 2018 22:42:18 -0400 Subject: [PATCH 01/19] Start of bulk-import method --- .gitignore | 1 + src/adapter/base.cr | 3 +++ src/adapter/mysql.cr | 25 +++++++++++++++++++++++++ src/granite_orm/transactions.cr | 14 +++++++++++++- 4 files changed, 42 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 7722c5d2..70cbbeb3 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ /.crystal/ /doc/ /config/*.db +/.idea/ # Libraries don't need dependency lock # Dependencies will be locked in application that uses them diff --git a/src/adapter/base.cr b/src/adapter/base.cr index 58cd3b97..ac058c5a 100644 --- a/src/adapter/base.cr +++ b/src/adapter/base.cr @@ -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) diff --git a/src/adapter/mysql.cr b/src/adapter/mysql.cr index fd3e08c8..53ce8fb2 100644 --- a/src/adapter/mysql.cr +++ b/src/adapter/mysql.cr @@ -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(", ") + 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 diff --git a/src/granite_orm/transactions.cr b/src/granite_orm/transactions.cr index b753e0e4..ed5a116f 100644 --- a/src/granite_orm/transactions.cr +++ b/src/granite_orm/transactions.cr @@ -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 } + 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. def save return false unless valid? From d1b2134eaf238a9e0035b6f7489f166e8141f8fd Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Fri, 30 Mar 2018 19:23:29 -0400 Subject: [PATCH 02/19] Use parameterized statement Add type filter to import class method Reformat some code --- .gitignore | 1 - src/adapter/mysql.cr | 33 +++++++++++++++++---------------- src/granite_orm/transactions.cr | 3 +-- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/.gitignore b/.gitignore index 70cbbeb3..7722c5d2 100644 --- a/.gitignore +++ b/.gitignore @@ -4,7 +4,6 @@ /.crystal/ /doc/ /config/*.db -/.idea/ # Libraries don't need dependency lock # Dependencies will be locked in application that uses them diff --git a/src/adapter/mysql.cr b/src/adapter/mysql.cr index 53ce8fb2..aebc8f7f 100644 --- a/src/adapter/mysql.cr +++ b/src/adapter/mysql.cr @@ -76,28 +76,29 @@ 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 + def import(table_name : String, primary_name : String, primary_auto : Bool, fields, model_array) + params = [] of DB::Any + 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| 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(", ") + stmt << '(' + stmt << "?," unless primary_auto + stmt << Array.new(fields.size, '?').join(',') + params << model.to_h[primary_name] unless primary_auto + params.concat fields.map { |field| model.to_h[field] } stmt << ")," end - end - end.chomp(",") + end.chomp(',') - log statement + log statement, params open do |db| - db.exec statement + db.exec statement, params end end diff --git a/src/granite_orm/transactions.cr b/src/granite_orm/transactions.cr index ed5a116f..a94ee14c 100644 --- a/src/granite_orm/transactions.cr +++ b/src/granite_orm/transactions.cr @@ -10,8 +10,7 @@ module Granite::ORM::Transactions # 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 } + def self.import(model_array : Array(self)) begin @@adapter.import(table_name, primary_name, {{primary_auto}}, fields, model_array) rescue err From 93b6e67aed15b7e3529c90f1c91631952ac3f94f Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Fri, 30 Mar 2018 23:15:34 -0400 Subject: [PATCH 03/19] Handle duplicate key IGNORE/UPDATE --- src/adapter/base.cr | 2 +- src/adapter/mysql.cr | 15 ++++++++++++--- src/granite_orm/transactions.cr | 4 ++-- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/adapter/base.cr b/src/adapter/base.cr index ac058c5a..1949042f 100644 --- a/src/adapter/base.cr +++ b/src/adapter/base.cr @@ -50,7 +50,7 @@ abstract class Granite::Adapter::Base 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) + abstract def import(table_name : String, primary_name : String, primary_auto : Bool, fields, model_array, **options) # This will update a row in the database. abstract def update(table_name, primary_name, fields, params) diff --git a/src/adapter/mysql.cr b/src/adapter/mysql.cr index aebc8f7f..492ffc70 100644 --- a/src/adapter/mysql.cr +++ b/src/adapter/mysql.cr @@ -76,10 +76,12 @@ class Granite::Adapter::Mysql < Granite::Adapter::Base end end - def import(table_name : String, primary_name : String, primary_auto : Bool, fields, model_array) + def import(table_name : String, primary_name : String, primary_auto : Bool, fields, model_array, **options) params = [] of DB::Any statement = String.build do |stmt| - stmt << "INSERT INTO #{quote(table_name)} (" + stmt << "INSERT" + stmt << " IGNORE" if options["on_duplicate_key_ignore"]? + stmt << " INTO #{quote(table_name)} (" stmt << "#{quote(primary_name)}, " unless primary_auto stmt << fields.map { |field| quote(field) }.join(", ") stmt << ") VALUES " @@ -95,7 +97,14 @@ class Granite::Adapter::Mysql < Granite::Adapter::Base end end.chomp(',') - log statement, params + if update_keys = options["on_duplicate_key_update"]? + statement += " ON DUPLICATE KEY UPDATE " + update_keys.each do |key| + statement += "#{quote(key)}=VALUES(#{quote(key)})" + end + end + + log statement, params open do |db| db.exec statement, params diff --git a/src/granite_orm/transactions.cr b/src/granite_orm/transactions.cr index a94ee14c..33f310d8 100644 --- a/src/granite_orm/transactions.cr +++ b/src/granite_orm/transactions.cr @@ -10,9 +10,9 @@ module Granite::ORM::Transactions # 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)) + def self.import(model_array : Array(self), **options) begin - @@adapter.import(table_name, primary_name, {{primary_auto}}, fields, model_array) + @@adapter.import(table_name, primary_name, {{primary_auto}}, fields, model_array, **options) rescue err raise DB::Error.new(err.message) end From 6435d633bc483f97d7368a14c60840ab80d42f29 Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Fri, 30 Mar 2018 23:22:24 -0400 Subject: [PATCH 04/19] Add commas between update values --- src/adapter/mysql.cr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/adapter/mysql.cr b/src/adapter/mysql.cr index 492ffc70..835917e3 100644 --- a/src/adapter/mysql.cr +++ b/src/adapter/mysql.cr @@ -100,8 +100,9 @@ class Granite::Adapter::Mysql < Granite::Adapter::Base if update_keys = options["on_duplicate_key_update"]? statement += " ON DUPLICATE KEY UPDATE " update_keys.each do |key| - statement += "#{quote(key)}=VALUES(#{quote(key)})" + statement += "#{quote(key)}=VALUES(#{quote(key)}), " end + statement = statement.chomp(", ") end log statement, params From 73e7de5d169c26f156c5c29b452c8cc266ff32a0 Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Sat, 31 Mar 2018 10:56:34 -0400 Subject: [PATCH 05/19] Remove now undeeded primary_auto variable --- src/adapter/base.cr | 2 +- src/adapter/mysql.cr | 5 +---- src/granite_orm/transactions.cr | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/adapter/base.cr b/src/adapter/base.cr index 1949042f..fb6357ef 100644 --- a/src/adapter/base.cr +++ b/src/adapter/base.cr @@ -50,7 +50,7 @@ abstract class Granite::Adapter::Base 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 : String, primary_name : String, primary_auto : Bool, fields, model_array, **options) + abstract def import(table_name : String, primary_name : String, fields, model_array, **options) # This will update a row in the database. abstract def update(table_name, primary_name, fields, params) diff --git a/src/adapter/mysql.cr b/src/adapter/mysql.cr index 835917e3..50cef7ef 100644 --- a/src/adapter/mysql.cr +++ b/src/adapter/mysql.cr @@ -76,22 +76,19 @@ class Granite::Adapter::Mysql < Granite::Adapter::Base end end - def import(table_name : String, primary_name : String, primary_auto : Bool, fields, model_array, **options) + def import(table_name : String, primary_name : String, fields, model_array, **options) params = [] of DB::Any statement = String.build do |stmt| stmt << "INSERT" stmt << " IGNORE" if options["on_duplicate_key_ignore"]? stmt << " 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| next unless model.valid? stmt << '(' - stmt << "?," unless primary_auto stmt << Array.new(fields.size, '?').join(',') - params << model.to_h[primary_name] unless primary_auto params.concat fields.map { |field| model.to_h[field] } stmt << ")," end diff --git a/src/granite_orm/transactions.cr b/src/granite_orm/transactions.cr index a0b0a6c1..e8177e21 100644 --- a/src/granite_orm/transactions.cr +++ b/src/granite_orm/transactions.cr @@ -12,7 +12,7 @@ module Granite::ORM::Transactions # invalid model records will be skipped def self.import(model_array : Array(self), **options) begin - @@adapter.import(table_name, primary_name, {{primary_auto}}, fields, model_array, **options) + @@adapter.import(table_name, primary_name, fields, model_array, **options) rescue err raise DB::Error.new(err.message) end From 1a03145ef4da03ed888a2bb38726de36e3bbe51d Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Sat, 31 Mar 2018 15:20:11 -0400 Subject: [PATCH 06/19] Add import for pg adapter --- src/adapter/pg.cr | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/adapter/pg.cr b/src/adapter/pg.cr index 0dafdea5..c99bc631 100644 --- a/src/adapter/pg.cr +++ b/src/adapter/pg.cr @@ -76,6 +76,42 @@ class Granite::Adapter::Pg < Granite::Adapter::Base end end + def import(table_name : String, primary_name : String, fields, model_array, **options) + params = [] of DB::Any + index = 0 + statement = String.build do |stmt| + stmt << "INSERT" + stmt << " INTO #{quote(table_name)} (" + stmt << fields.map { |field| quote(field) }.join(", ") + stmt << ") VALUES " + + model_array.each do |model| + next unless model.valid? + stmt << '(' + stmt << fields.map_with_index { |_f, idx| "$#{index+idx+1}" }.join(',') + params.concat fields.map { |field| model.to_h[field] } + stmt << ")," + index += fields.size + end + end.chomp(',') + + if update_keys = options["on_duplicate_key_update"]? + statement += " ON CONFLICT (#{quote(primary_name)}) DO UPDATE SET " + update_keys.each do |key| + statement += "#{quote(key)}=EXCLUDED.#{key}, " + end + statement = statement.chomp(", ") + elsif options["on_duplicate_key_ignore"]? + statement += " ON CONFLICT DO NOTHING" + end + + log statement, params + + open do |db| + db.exec statement, params + end + end + private def last_val return "SELECT LASTVAL()" end From 54dd5b7cb94a87daf174b30899ebdee8ce609ed6 Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Sat, 31 Mar 2018 15:21:33 -0400 Subject: [PATCH 07/19] Also quote other column key --- src/adapter/pg.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/adapter/pg.cr b/src/adapter/pg.cr index c99bc631..1dc5388b 100644 --- a/src/adapter/pg.cr +++ b/src/adapter/pg.cr @@ -98,7 +98,7 @@ class Granite::Adapter::Pg < Granite::Adapter::Base if update_keys = options["on_duplicate_key_update"]? statement += " ON CONFLICT (#{quote(primary_name)}) DO UPDATE SET " update_keys.each do |key| - statement += "#{quote(key)}=EXCLUDED.#{key}, " + statement += "#{quote(key)}=EXCLUDED.#{quote(key)}, " end statement = statement.chomp(", ") elsif options["on_duplicate_key_ignore"]? From c74d12694cd7ae9a1c1354a0508245c6419ee54c Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Sat, 31 Mar 2018 19:59:15 -0400 Subject: [PATCH 08/19] Add adapter for sqlite Handle timestamps Start of specs Ran crystal tool format --- spec/granite_orm/fields/timestamps_spec.cr | 2 +- spec/granite_orm/querying/find_spec.cr | 2 +- spec/granite_orm/transactions/import_spec.cr | 17 +++++++++ spec/spec_helper.cr | 2 +- src/adapter/mysql.cr | 9 +++-- src/adapter/pg.cr | 11 ++++-- src/adapter/sqlite.cr | 38 ++++++++++++++++++++ src/granite_orm/associations.cr | 2 +- src/granite_orm/querying.cr | 2 +- src/granite_orm/transactions.cr | 2 +- src/granite_orm/validators.cr | 2 +- 11 files changed, 77 insertions(+), 12 deletions(-) create mode 100644 spec/granite_orm/transactions/import_spec.cr diff --git a/spec/granite_orm/fields/timestamps_spec.cr b/spec/granite_orm/fields/timestamps_spec.cr index 4447de55..4a2dd07b 100644 --- a/spec/granite_orm/fields/timestamps_spec.cr +++ b/spec/granite_orm/fields/timestamps_spec.cr @@ -5,7 +5,7 @@ require "../../spec_helper" module {{adapter.capitalize.id}} {% avoid_macro_bug = 1 # https://github.com/crystal-lang/crystal/issues/5724 - + # TODO mysql timestamp support should work better if adapter == "pg" time_kind_on_read = "Time::Kind::Utc".id diff --git a/spec/granite_orm/querying/find_spec.cr b/spec/granite_orm/querying/find_spec.cr index 35bcd223..ce4f50f1 100644 --- a/spec/granite_orm/querying/find_spec.cr +++ b/spec/granite_orm/querying/find_spec.cr @@ -60,7 +60,7 @@ module {{adapter.capitalize.id}} it "returns nil or raises if no result" do found = Parent.find? 0 found.should be_nil - + expect_raises(Granite::ORM::Querying::NotFound, /Couldn't find .*Parent.* with id=0/) do Parent.find 0 end diff --git a/spec/granite_orm/transactions/import_spec.cr b/spec/granite_orm/transactions/import_spec.cr new file mode 100644 index 00000000..d9fe47c5 --- /dev/null +++ b/spec/granite_orm/transactions/import_spec.cr @@ -0,0 +1,17 @@ +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 + end +end +{% end %} diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 988181e7..d3149ab2 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -1,7 +1,7 @@ require "spec" module GraniteExample - ADAPTERS = ["pg","mysql","sqlite"] + ADAPTERS = ["pg", "mysql", "sqlite"] end require "../src/granite_orm" diff --git a/src/adapter/mysql.cr b/src/adapter/mysql.cr index 50cef7ef..093de203 100644 --- a/src/adapter/mysql.cr +++ b/src/adapter/mysql.cr @@ -77,7 +77,10 @@ class Granite::Adapter::Mysql < Granite::Adapter::Base end def import(table_name : String, primary_name : String, fields, model_array, **options) - params = [] of DB::Any + params = [] of DB::Any + now = Time.now.to_utc + fields.reject! { |field| field === "id" } if primary_name === "id" + statement = String.build do |stmt| stmt << "INSERT" stmt << " IGNORE" if options["on_duplicate_key_ignore"]? @@ -86,6 +89,8 @@ class Granite::Adapter::Mysql < Granite::Adapter::Base stmt << ") VALUES " model_array.each do |model| + model.updated_at = now if model.responds_to? :updated_at + model.created_at = now if model.responds_to? :created_at next unless model.valid? stmt << '(' stmt << Array.new(fields.size, '?').join(',') @@ -102,7 +107,7 @@ class Granite::Adapter::Mysql < Granite::Adapter::Base statement = statement.chomp(", ") end - log statement, params + log statement, params open do |db| db.exec statement, params diff --git a/src/adapter/pg.cr b/src/adapter/pg.cr index 1dc5388b..da468fb5 100644 --- a/src/adapter/pg.cr +++ b/src/adapter/pg.cr @@ -78,7 +78,10 @@ class Granite::Adapter::Pg < Granite::Adapter::Base def import(table_name : String, primary_name : String, fields, model_array, **options) params = [] of DB::Any + now = Time.now.to_utc + fields.reject! { |field| field === "id" } if primary_name === "id" index = 0 + statement = String.build do |stmt| stmt << "INSERT" stmt << " INTO #{quote(table_name)} (" @@ -86,9 +89,11 @@ class Granite::Adapter::Pg < Granite::Adapter::Base stmt << ") VALUES " model_array.each do |model| + model.updated_at = now if model.responds_to? :updated_at + model.created_at = now if model.responds_to? :created_at next unless model.valid? stmt << '(' - stmt << fields.map_with_index { |_f, idx| "$#{index+idx+1}" }.join(',') + stmt << fields.map_with_index { |_f, idx| "$#{index + idx + 1}" }.join(',') params.concat fields.map { |field| model.to_h[field] } stmt << ")," index += fields.size @@ -105,10 +110,10 @@ class Granite::Adapter::Pg < Granite::Adapter::Base statement += " ON CONFLICT DO NOTHING" end - log statement, params + log statement, params open do |db| - db.exec statement, params + db.exec statement, params end end diff --git a/src/adapter/sqlite.cr b/src/adapter/sqlite.cr index 27de2cf0..eab211f5 100644 --- a/src/adapter/sqlite.cr +++ b/src/adapter/sqlite.cr @@ -74,6 +74,44 @@ class Granite::Adapter::Sqlite < Granite::Adapter::Base end end + def import(table_name : String, primary_name : String, fields, model_array, **options) + params = [] of DB::Any + now = Time.now.to_utc + fields.reject! { |field| field === "id" } if primary_name === "id" + + statement = String.build do |stmt| + stmt << "INSERT" + stmt << " IGNORE" if options["on_duplicate_key_ignore"]? + stmt << " INTO #{quote(table_name)} (" + stmt << fields.map { |field| quote(field) }.join(", ") + stmt << ") VALUES " + + model_array.each do |model| + next unless model.valid? + model.updated_at = now if model.responds_to? :updated_at + model.created_at = now if model.responds_to? :created_at + stmt << '(' + stmt << Array.new(fields.size, '?').join(',') + params.concat fields.map { |field| model.to_h[field] } + stmt << ")," + end + end.chomp(',') + + if update_keys = options["on_duplicate_key_update"]? + statement += " ON DUPLICATE KEY UPDATE " + update_keys.each do |key| + statement += "#{quote(key)}=VALUES(#{quote(key)}), " + end + statement = statement.chomp(", ") + end + + log statement, params + + open do |db| + db.exec statement, params + end + end + private def last_val return "SELECT LAST_INSERT_ROWID()" end diff --git a/src/granite_orm/associations.cr b/src/granite_orm/associations.cr index d52d9dd7..9fa28736 100644 --- a/src/granite_orm/associations.cr +++ b/src/granite_orm/associations.cr @@ -33,7 +33,7 @@ module Granite::ORM::Associations {{children_class}}.all(query, id) end end - + # define getter for related children macro has_many(children_table, through) def {{children_table.id}} diff --git a/src/granite_orm/querying.cr b/src/granite_orm/querying.cr index 70a5b8bd..fe6dd400 100644 --- a/src/granite_orm/querying.cr +++ b/src/granite_orm/querying.cr @@ -1,6 +1,6 @@ module Granite::ORM::Querying class NotFound < Exception - end + end macro extended macro __process_querying diff --git a/src/granite_orm/transactions.cr b/src/granite_orm/transactions.cr index e8177e21..e6ce220b 100644 --- a/src/granite_orm/transactions.cr +++ b/src/granite_orm/transactions.cr @@ -102,7 +102,7 @@ module Granite::ORM::Transactions end end - module ClassMethods + module ClassMethods def create(**args) create(args.to_h) end diff --git a/src/granite_orm/validators.cr b/src/granite_orm/validators.cr index bbef57dd..fcbc2ea4 100644 --- a/src/granite_orm/validators.cr +++ b/src/granite_orm/validators.cr @@ -8,7 +8,7 @@ require "./error" # !user.name.to_s.blank? # end # -# validate :name, "can't be blank", -> (user : User) do +# validate :name, "can't be blank", ->(user : User) do # !user.name.to_s.blank? # end # From 84364d918efb58edb016f63646531a74c0a71b40 Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Fri, 6 Apr 2018 21:03:43 -0400 Subject: [PATCH 09/19] Fix spec --- src/granite_orm/transactions.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/granite_orm/transactions.cr b/src/granite_orm/transactions.cr index e6ce220b..d2dc361c 100644 --- a/src/granite_orm/transactions.cr +++ b/src/granite_orm/transactions.cr @@ -12,7 +12,7 @@ module Granite::ORM::Transactions # invalid model records will be skipped def self.import(model_array : Array(self), **options) begin - @@adapter.import(table_name, primary_name, fields, model_array, **options) + @@adapter.import(table_name, primary_name, fields.dup, model_array, **options) rescue err raise DB::Error.new(err.message) end From 2d15769215402d94ae6b06d7fcae19ab3f5ee5a9 Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Fri, 6 Apr 2018 21:38:43 -0400 Subject: [PATCH 10/19] Add more tests, fix sqlite adapter --- spec/granite_orm/transactions/import_spec.cr | 44 +++++++++++++++++++- src/adapter/sqlite.cr | 14 +++---- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/spec/granite_orm/transactions/import_spec.cr b/spec/granite_orm/transactions/import_spec.cr index d9fe47c5..89f6ff42 100644 --- a/spec/granite_orm/transactions/import_spec.cr +++ b/spec/granite_orm/transactions/import_spec.cr @@ -11,7 +11,49 @@ module {{adapter.capitalize.id}} ] 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, on_duplicate_key_update: ["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, on_duplicate_key_ignore: true) + + if parent = Parent.find 113 + parent.name.should be "ImportParent3" + parent.id.should eq 113 + end end - end + end end {% end %} diff --git a/src/adapter/sqlite.cr b/src/adapter/sqlite.cr index eab211f5..b8c4b857 100644 --- a/src/adapter/sqlite.cr +++ b/src/adapter/sqlite.cr @@ -81,7 +81,11 @@ class Granite::Adapter::Sqlite < Granite::Adapter::Base statement = String.build do |stmt| stmt << "INSERT" - stmt << " IGNORE" if options["on_duplicate_key_ignore"]? + if options["on_duplicate_key_update"]? + stmt << " OR REPLACE " + elsif options["on_duplicate_key_ignore"]? + stmt << " OR IGNORE " + end stmt << " INTO #{quote(table_name)} (" stmt << fields.map { |field| quote(field) }.join(", ") stmt << ") VALUES " @@ -97,14 +101,6 @@ class Granite::Adapter::Sqlite < Granite::Adapter::Base end end.chomp(',') - if update_keys = options["on_duplicate_key_update"]? - statement += " ON DUPLICATE KEY UPDATE " - update_keys.each do |key| - statement += "#{quote(key)}=VALUES(#{quote(key)}), " - end - statement = statement.chomp(", ") - end - log statement, params open do |db| From bbd2f7f3c7f77664c75cecf5a5129c3dfe1033ef Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Fri, 6 Apr 2018 21:54:48 -0400 Subject: [PATCH 11/19] Update readme --- README.md | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/README.md b/README.md index 11f1638e..84d79384 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,69 @@ class Site < Granite::Base end ``` +### Bulk Insertions + +#### Import + +Each model has an `import` class level method to import an array of models in one bulk insert statement. +```Crystal +models [ + Model.new(id: 1, name: "Fred", age: 14), + Model.new(i1: 2, name: "Joe", age 25), + Model.new(id: 3, name: "John", 30), +] + +Model.import(models) +``` + +#### on_duplicate_key_update + +The `import` method has an optional `on_duplicate_key_update` param allows you to specify fields (as an array of strings) to be updated should a primary constraint is violated. +```Crystal +models [ + Model.new(id: 1, name: "Fred", age: 14), + Model.new(i1: 2, name: "Joe", age 25), + Model.new(id: 3, name: "John", 30), +] + +Model.import(models) + +Model.find!(1).name # => Fred + +models [ + Model.new(id: 1, name: "George", age: 14), +] + +Model.import(models, on_duplicate_key_update: %w(name)) + +Model.find!(1).name # => George +``` + +##### NOTE: If using PostgreSQL you must have version 9.5+ to have the on_duplicate_key_update feature. + +#### on_duplicate_key_ignore + +the `import` method has an optional `on_duplicate_key_ignore` param, that takes a boolean, which will skip records if the primary constraint is violated. +```Crystal +models [ + Model.new(id: 1, name: "Fred", age: 14), + Model.new(i1: 2, name: "Joe", age 25), + Model.new(id: 3, name: "John", 30), +] + +Model.import(models) + +Model.find!(1).name # => Fred + +models [ + Model.new(id: 1, name: "George", age: 14), +] + +Model.import(models, on_duplicate_key_ignore: true) + +Model.find!(1).name # => Fred +``` + ### SQL To clear all the rows in the database: From 2425ad25b99e46cbab354cf86940d0e1f6202ff4 Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Fri, 6 Apr 2018 21:59:41 -0400 Subject: [PATCH 12/19] Fix spacing issue in sqlite adapater --- src/adapter/sqlite.cr | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/adapter/sqlite.cr b/src/adapter/sqlite.cr index b8c4b857..a3b02890 100644 --- a/src/adapter/sqlite.cr +++ b/src/adapter/sqlite.cr @@ -80,13 +80,13 @@ class Granite::Adapter::Sqlite < Granite::Adapter::Base fields.reject! { |field| field === "id" } if primary_name === "id" statement = String.build do |stmt| - stmt << "INSERT" + stmt << "INSERT " if options["on_duplicate_key_update"]? - stmt << " OR REPLACE " + stmt << "OR REPLACE " elsif options["on_duplicate_key_ignore"]? - stmt << " OR IGNORE " + stmt << "OR IGNORE " end - stmt << " INTO #{quote(table_name)} (" + stmt << "INTO #{quote(table_name)} (" stmt << fields.map { |field| quote(field) }.join(", ") stmt << ") VALUES " From 87009d530230735589716b3b4b79cc4e63f2bfb6 Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Fri, 6 Apr 2018 22:31:53 -0400 Subject: [PATCH 13/19] Fix indentation --- src/granite_orm/transactions.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/granite_orm/transactions.cr b/src/granite_orm/transactions.cr index d2dc361c..4a90f476 100644 --- a/src/granite_orm/transactions.cr +++ b/src/granite_orm/transactions.cr @@ -12,7 +12,7 @@ module Granite::ORM::Transactions # invalid model records will be skipped def self.import(model_array : Array(self), **options) begin - @@adapter.import(table_name, primary_name, fields.dup, model_array, **options) + @@adapter.import(table_name, primary_name, fields.dup, model_array, **options) rescue err raise DB::Error.new(err.message) end From acc4a6e9d40937b4071d6fd5cc1f5b85bec7d0b0 Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Fri, 6 Apr 2018 22:47:44 -0400 Subject: [PATCH 14/19] Fix typos in readme examples --- README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 84d79384..3740dc45 100644 --- a/README.md +++ b/README.md @@ -124,8 +124,8 @@ Each model has an `import` class level method to import an array of models in on ```Crystal models [ Model.new(id: 1, name: "Fred", age: 14), - Model.new(i1: 2, name: "Joe", age 25), - Model.new(id: 3, name: "John", 30), + Model.new(id: 2, name: "Joe", age: 25), + Model.new(id: 3, name: "John", age: 30), ] Model.import(models) @@ -137,8 +137,8 @@ The `import` method has an optional `on_duplicate_key_update` param allows you t ```Crystal models [ Model.new(id: 1, name: "Fred", age: 14), - Model.new(i1: 2, name: "Joe", age 25), - Model.new(id: 3, name: "John", 30), + Model.new(id: 2, name: "Joe", age: 25), + Model.new(id: 3, name: "John", age: 30), ] Model.import(models) @@ -162,8 +162,8 @@ the `import` method has an optional `on_duplicate_key_ignore` param, that takes ```Crystal models [ Model.new(id: 1, name: "Fred", age: 14), - Model.new(i1: 2, name: "Joe", age 25), - Model.new(id: 3, name: "John", 30), + Model.new(id: 2, name: "Joe", age: 25), + Model.new(id: 3, name: "John", age: 30), ] Model.import(models) From 87268a0741db8656894cf57f00db59e8e08240ec Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Sat, 7 Apr 2018 13:24:00 -0400 Subject: [PATCH 15/19] Fix indentations --- spec/granite_orm/transactions/import_spec.cr | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/granite_orm/transactions/import_spec.cr b/spec/granite_orm/transactions/import_spec.cr index 89f6ff42..c0bfcb07 100644 --- a/spec/granite_orm/transactions/import_spec.cr +++ b/spec/granite_orm/transactions/import_spec.cr @@ -11,7 +11,7 @@ module {{adapter.capitalize.id}} ] Parent.import(to_import) Parent.all("WHERE name LIKE ?", ["Import%"]).size.should eq 3 - end + end it "should work with on_duplicate_key_update" do to_import = [ @@ -30,9 +30,9 @@ module {{adapter.capitalize.id}} if parent = Parent.find 112 parent.name.should be "ImportParent112" - parent.id.should eq 112 - end - end + parent.id.should eq 112 + end + end it "should work with on_duplicate_key_ignore" do to_import = [ @@ -51,9 +51,9 @@ module {{adapter.capitalize.id}} if parent = Parent.find 113 parent.name.should be "ImportParent3" - parent.id.should eq 113 + parent.id.should eq 113 end end - end + end end {% end %} From f6dcf2adbda512bb61722741f2316593465bc8f8 Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Sat, 7 Apr 2018 21:35:54 -0400 Subject: [PATCH 16/19] Add overload methods for update/ignore_on_duplicate, update readme --- README.md | 12 ++++++------ spec/granite_orm/transactions/import_spec.cr | 4 ++-- src/adapter/mysql.cr | 14 ++++++++------ src/adapter/pg.cr | 12 +++++++----- src/adapter/sqlite.cr | 4 ++-- src/granite_orm/transactions.cr | 20 ++++++++++++++++++-- 6 files changed, 43 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index d4d69ce3..1f1891fa 100644 --- a/README.md +++ b/README.md @@ -131,9 +131,9 @@ models [ Model.import(models) ``` -#### on_duplicate_key_update +#### update_on_duplicate -The `import` method has an optional `on_duplicate_key_update` param allows you to specify fields (as an array of strings) to be updated should a primary constraint is violated. +The `import` method has an optional `update_on_duplicate` + `columns` params that allows you to specify the columns (as an array of strings) that should be updated if primary constraint is violated. ```Crystal models [ Model.new(id: 1, name: "Fred", age: 14), @@ -149,16 +149,16 @@ models [ Model.new(id: 1, name: "George", age: 14), ] -Model.import(models, on_duplicate_key_update: %w(name)) +Model.import(models, update_on_duplicate: true, columns: %w(name)) Model.find!(1).name # => George ``` ##### NOTE: If using PostgreSQL you must have version 9.5+ to have the on_duplicate_key_update feature. -#### on_duplicate_key_ignore +#### ignore_on_duplicate -the `import` method has an optional `on_duplicate_key_ignore` param, that takes a boolean, which will skip records if the primary constraint is violated. +the `import` method has an optional `ignore_on_duplicate` param, that takes a boolean, which will skip records if the primary constraint is violated. ```Crystal models [ Model.new(id: 1, name: "Fred", age: 14), @@ -174,7 +174,7 @@ models [ Model.new(id: 1, name: "George", age: 14), ] -Model.import(models, on_duplicate_key_ignore: true) +Model.import(models, ignore_on_duplicate: true) Model.find!(1).name # => Fred ``` diff --git a/spec/granite_orm/transactions/import_spec.cr b/spec/granite_orm/transactions/import_spec.cr index c0bfcb07..904573ca 100644 --- a/spec/granite_orm/transactions/import_spec.cr +++ b/spec/granite_orm/transactions/import_spec.cr @@ -26,7 +26,7 @@ module {{adapter.capitalize.id}} Parent.new(id: 112, name: "ImportParent112"), ] - Parent.import(to_import, on_duplicate_key_update: ["name"]) + Parent.import(to_import, update_on_duplicate: true, columns: ["name"]) if parent = Parent.find 112 parent.name.should be "ImportParent112" @@ -47,7 +47,7 @@ module {{adapter.capitalize.id}} Parent.new(id: 113, name: "ImportParent113"), ] - Parent.import(to_import, on_duplicate_key_ignore: true) + Parent.import(to_import, ignore_on_duplicate: true) if parent = Parent.find 113 parent.name.should be "ImportParent3" diff --git a/src/adapter/mysql.cr b/src/adapter/mysql.cr index 093de203..abdce23e 100644 --- a/src/adapter/mysql.cr +++ b/src/adapter/mysql.cr @@ -83,7 +83,7 @@ class Granite::Adapter::Mysql < Granite::Adapter::Base statement = String.build do |stmt| stmt << "INSERT" - stmt << " IGNORE" if options["on_duplicate_key_ignore"]? + stmt << " IGNORE" if options["ignore_on_duplicate"]? stmt << " INTO #{quote(table_name)} (" stmt << fields.map { |field| quote(field) }.join(", ") stmt << ") VALUES " @@ -99,12 +99,14 @@ class Granite::Adapter::Mysql < Granite::Adapter::Base end end.chomp(',') - if update_keys = options["on_duplicate_key_update"]? - statement += " ON DUPLICATE KEY UPDATE " - update_keys.each do |key| - statement += "#{quote(key)}=VALUES(#{quote(key)}), " + if options["update_on_duplicate"]? + if columns = options["columns"]? + statement += " ON DUPLICATE KEY UPDATE " + columns.each do |key| + statement += "#{quote(key)}=VALUES(#{quote(key)}), " + end + statement = statement.chomp(", ") end - statement = statement.chomp(", ") end log statement, params diff --git a/src/adapter/pg.cr b/src/adapter/pg.cr index da468fb5..6379259d 100644 --- a/src/adapter/pg.cr +++ b/src/adapter/pg.cr @@ -100,13 +100,15 @@ class Granite::Adapter::Pg < Granite::Adapter::Base end end.chomp(',') - if update_keys = options["on_duplicate_key_update"]? - statement += " ON CONFLICT (#{quote(primary_name)}) DO UPDATE SET " - update_keys.each do |key| - statement += "#{quote(key)}=EXCLUDED.#{quote(key)}, " + if options["update_on_duplicate"]? + if columns = options["columns"]? + statement += " ON CONFLICT (#{quote(primary_name)}) DO UPDATE SET " + columns.each do |key| + statement += "#{quote(key)}=EXCLUDED.#{quote(key)}, " + end end statement = statement.chomp(", ") - elsif options["on_duplicate_key_ignore"]? + elsif options["ignore_on_duplicate"]? statement += " ON CONFLICT DO NOTHING" end diff --git a/src/adapter/sqlite.cr b/src/adapter/sqlite.cr index a3b02890..e6b1d756 100644 --- a/src/adapter/sqlite.cr +++ b/src/adapter/sqlite.cr @@ -81,9 +81,9 @@ class Granite::Adapter::Sqlite < Granite::Adapter::Base statement = String.build do |stmt| stmt << "INSERT " - if options["on_duplicate_key_update"]? + if options["update_on_duplicate"]? stmt << "OR REPLACE " - elsif options["on_duplicate_key_ignore"]? + elsif options["ignore_on_duplicate"]? stmt << "OR IGNORE " end stmt << "INTO #{quote(table_name)} (" diff --git a/src/granite_orm/transactions.cr b/src/granite_orm/transactions.cr index 4a90f476..2e3dc831 100644 --- a/src/granite_orm/transactions.cr +++ b/src/granite_orm/transactions.cr @@ -10,9 +10,25 @@ module Granite::ORM::Transactions # 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), **options) + def self.import(model_array : Array(self)) begin - @@adapter.import(table_name, primary_name, fields.dup, model_array, **options) + @@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)) + 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 From e3e525aca4616acbfb60fe68487b043321590a5c Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Thu, 12 Apr 2018 16:24:46 -0400 Subject: [PATCH 17/19] Add = sign to model arrays in readme --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 1f1891fa..6bdb0cdc 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,7 @@ end Each model has an `import` class level method to import an array of models in one bulk insert statement. ```Crystal -models [ +models = [ Model.new(id: 1, name: "Fred", age: 14), Model.new(id: 2, name: "Joe", age: 25), Model.new(id: 3, name: "John", age: 30), @@ -135,7 +135,7 @@ Model.import(models) The `import` method has an optional `update_on_duplicate` + `columns` params that allows you to specify the columns (as an array of strings) that should be updated if primary constraint is violated. ```Crystal -models [ +models = [ Model.new(id: 1, name: "Fred", age: 14), Model.new(id: 2, name: "Joe", age: 25), Model.new(id: 3, name: "John", age: 30), @@ -145,7 +145,7 @@ Model.import(models) Model.find!(1).name # => Fred -models [ +models = [ Model.new(id: 1, name: "George", age: 14), ] @@ -160,7 +160,7 @@ Model.find!(1).name # => George the `import` method has an optional `ignore_on_duplicate` param, that takes a boolean, which will skip records if the primary constraint is violated. ```Crystal -models [ +models = [ Model.new(id: 1, name: "Fred", age: 14), Model.new(id: 2, name: "Joe", age: 25), Model.new(id: 3, name: "John", age: 30), @@ -170,7 +170,7 @@ Model.import(models) Model.find!(1).name # => Fred -models [ +models = [ Model.new(id: 1, name: "George", age: 14), ] From 9fe4bcb8bab75bfd1a3e2d889ae06bf115d10b5d Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Wed, 25 Apr 2018 00:15:29 -0400 Subject: [PATCH 18/19] Specify imports do not trigger callbacks yet --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 6bdb0cdc..2eb6092f 100644 --- a/README.md +++ b/README.md @@ -120,6 +120,8 @@ end #### Import +**Note: As of now imports do not trigger callbacks.** + Each model has an `import` class level method to import an array of models in one bulk insert statement. ```Crystal models = [ From 5b31b80cbdb4da650c10c7bf65419f1cdc4e05d3 Mon Sep 17 00:00:00 2001 From: Blacksmoke16 Date: Wed, 25 Apr 2018 00:16:16 -0400 Subject: [PATCH 19/19] proper grammar --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2eb6092f..e7894c4f 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ end #### Import -**Note: As of now imports do not trigger callbacks.** +**Note: As of now, imports do not trigger callbacks.** Each model has an `import` class level method to import an array of models in one bulk insert statement. ```Crystal