Skip to content

Commit 3f88a74

Browse files
committed
extend "query_requires_identity_insert?" for merge queries with inserts
1 parent c0be62c commit 3f88a74

File tree

4 files changed

+41
-14
lines changed

4 files changed

+41
-14
lines changed

lib/active_record/connection_adapters/sqlserver/database_statements.rb

+10-13
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,6 @@ def build_insert_sql(insert) # :nodoc:
161161
# if we do not have any columns that might have conflicting values, just execute a regular insert
162162
return build_sql_for_regular_insert(insert) if conflict_columns.flatten.empty?
163163

164-
primary_keys_for_insert = insert_all.primary_keys.to_set
165-
166-
# if we receive a composite primary key, MSSQL will not be happy when we want to call "IDENTITY_INSERT"
167-
# as there is likely no IDENTITY column
168-
# so we need to check if there is exacty one
169-
# TODO: Refactor to use existing "SET IDENTITY_INSERT" settings
170-
enable_identity_insert = primary_keys_for_insert.length == 1 &&
171-
(insert_all.primary_keys.to_set & insert.keys).present?
172-
173164
# why is the "PARTITION BY" clause needed?
174165
# in every DBMS system, insert_all / upsert_all is usually implemented with INSERT, that allows to define what happens
175166
# when duplicates are found (SKIP OR UPDATE)
@@ -179,7 +170,7 @@ def build_insert_sql(insert) # :nodoc:
179170
# this works easiest by using PARTITION and make sure that any record
180171
# we are trying to insert is "the first one seen across all the potential conflicted columns"
181172
sql = <<~SQL
182-
#{"SET IDENTITY_INSERT #{insert.model.quoted_table_name} ON;" if enable_identity_insert}
173+
183174
MERGE INTO #{insert.model.quoted_table_name} WITH (UPDLOCK, HOLDLOCK) AS target
184175
USING (
185176
SELECT *
@@ -230,7 +221,6 @@ def build_insert_sql(insert) # :nodoc:
230221
sql << build_sql_for_returning(insert:, insert_all: insert.send(:insert_all))
231222

232223
sql << ";"
233-
sql << "SET IDENTITY_INSERT #{insert.model.quoted_table_name} OFF;" if enable_identity_insert
234224

235225
return sql
236226
end
@@ -527,11 +517,18 @@ def query_requires_identity_insert?(sql)
527517
raw_table_name = get_raw_table_name(sql)
528518
id_column = identity_columns(raw_table_name).first
529519

530-
(id_column && sql =~ /^\s*(INSERT|EXEC sp_executesql N'INSERT)[^(]+\([^)]*\b(#{id_column.name})\b,?[^)]*\)/i) ? SQLServer::Utils.extract_identifiers(raw_table_name).quoted : false
520+
if id_column && (
521+
sql =~ /^\s*(INSERT|EXEC sp_executesql N'INSERT)[^(]+\([^)]*\b(#{id_column.name})\b,?[^)]*\)/i ||
522+
sql =~ /^\s*MERGE INTO.+THEN INSERT \([^)]*\b(#{id_column.name})\b,?[^)]*\)/im
523+
)
524+
SQLServer::Utils.extract_identifiers(raw_table_name).quoted
525+
else
526+
false
527+
end
531528
end
532529

533530
def insert_sql?(sql)
534-
!(sql =~ /\A\s*(INSERT|EXEC sp_executesql N'INSERT)/i).nil?
531+
!(sql =~ /\A\s*(INSERT|EXEC sp_executesql N'INSERT|MERGE INTO.+THEN INSERT)/im).nil?
535532
end
536533

537534
def identity_columns(table_name)

lib/active_record/connection_adapters/sqlserver/schema_statements.rb

+2
Original file line numberDiff line numberDiff line change
@@ -759,6 +759,8 @@ def get_raw_table_name(sql)
759759
.match(/\s*([^(]*)/i)[0]
760760
elsif s.match?(/^\s*UPDATE\s+.*/i)
761761
s.match(/UPDATE\s+([^\(\s]+)\s*/i)[1]
762+
elsif s.match?(/^\s*MERGE INTO.*/i)
763+
s.match(/^\s*MERGE\s+INTO\s+(\[?[a-z_ -]+\]?\.?\[?[a-z_ -]+\]?)\s+(AS|WITH|USING)/i)[1]
762764
else
763765
s.match(/FROM[\s|\(]+((\[[^\(\]]+\])|[^\(\s]+)\s*/i)[1]
764766
end.strip

test/cases/adapter_test_sqlserver.rb

+11-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class AdapterTestSQLServer < ActiveRecord::TestCase
1313
fixtures :tasks
1414

1515
let(:basic_insert_sql) { "INSERT INTO [funny_jokes] ([name]) VALUES('Knock knock')" }
16+
let(:basic_merge_sql) { "MERGE INTO [ships] WITH (UPDLOCK, HOLDLOCK) AS target USING ( SELECT * FROM ( SELECT [id], [name], ROW_NUMBER() OVER ( PARTITION BY [id] ORDER BY [id] DESC ) AS rn_0 FROM ( VALUES (101, N'RSS Sir David Attenborough') ) AS t1 ([id], [name]) ) AS ranked_source WHERE rn_0 = 1 ) AS source ON (target.[id] = source.[id]) WHEN MATCHED THEN UPDATE SET target.[name] = source.[name]" }
1617
let(:basic_update_sql) { "UPDATE [customers] SET [address_street] = NULL WHERE [id] = 2" }
1718
let(:basic_select_sql) { "SELECT * FROM [customers] WHERE ([customers].[id] = 1)" }
1819

@@ -91,6 +92,7 @@ class AdapterTestSQLServer < ActiveRecord::TestCase
9192

9293
it "return unquoted table name object from basic INSERT UPDATE and SELECT statements" do
9394
assert_equal "funny_jokes", connection.send(:get_table_name, basic_insert_sql)
95+
assert_equal "ships", connection.send(:get_table_name, basic_merge_sql)
9496
assert_equal "customers", connection.send(:get_table_name, basic_update_sql)
9597
assert_equal "customers", connection.send(:get_table_name, basic_select_sql)
9698
end
@@ -219,6 +221,10 @@ class AdapterTestSQLServer < ActiveRecord::TestCase
219221
@identity_insert_sql_unquoted_sp = "EXEC sp_executesql N'INSERT INTO funny_jokes (id, name) VALUES (@0, @1)', N'@0 int, @1 nvarchar(255)', @0 = 420, @1 = N'Knock knock'"
220222
@identity_insert_sql_unordered_sp = "EXEC sp_executesql N'INSERT INTO [funny_jokes] ([name],[id]) VALUES (@0, @1)', N'@0 nvarchar(255), @1 int', @0 = N'Knock knock', @1 = 420"
221223

224+
@identity_merge_sql = "MERGE INTO [ships] WITH (UPDLOCK, HOLDLOCK) AS target USING ( SELECT * FROM ( SELECT [id], [name], ROW_NUMBER() OVER ( PARTITION BY [id] ORDER BY [id] DESC ) AS rn_0 FROM ( VALUES (101, N'RSS Sir David Attenborough') ) AS t1 ([id], [name]) ) AS ranked_source WHERE rn_0 = 1 ) AS source ON (target.[id] = source.[id]) WHEN MATCHED THEN UPDATE SET target.[name] = source.[name] WHEN NOT MATCHED BY TARGET THEN INSERT ([id], [name]) VALUES (source.[id], source.[name]) OUTPUT INSERTED.[id]"
225+
@identity_merge_sql_unquoted = "MERGE INTO ships WITH (UPDLOCK, HOLDLOCK) AS target USING ( SELECT * FROM ( SELECT id, name, ROW_NUMBER() OVER ( PARTITION BY id ORDER BY id DESC ) AS rn_0 FROM ( VALUES (101, N'RSS Sir David Attenborough') ) AS t1 (id, name) ) AS ranked_source WHERE rn_0 = 1 ) AS source ON (target.id = source.id) WHEN MATCHED THEN UPDATE SET target.name = source.name WHEN NOT MATCHED BY TARGET THEN INSERT (id, name) VALUES (source.id, source.name) OUTPUT INSERTED.id"
226+
@identity_merge_sql_unordered = "MERGE INTO [ships] WITH (UPDLOCK, HOLDLOCK) AS target USING ( SELECT * FROM ( SELECT [name], [id], ROW_NUMBER() OVER ( PARTITION BY [id] ORDER BY [id] DESC ) AS rn_0 FROM ( VALUES (101, N'RSS Sir David Attenborough') ) AS t1 ([name], [id]) ) AS ranked_source WHERE rn_0 = 1 ) AS source ON (target.[id] = source.[id]) WHEN MATCHED THEN UPDATE SET target.[name] = source.[name] WHEN NOT MATCHED BY TARGET THEN INSERT ([name], [id]) VALUES (source.[name], source.[id]) OUTPUT INSERTED.[id]"
227+
222228
@identity_insert_sql_non_dbo = "INSERT INTO [test].[aliens] ([id],[name]) VALUES(420,'Mork')"
223229
@identity_insert_sql_non_dbo_unquoted = "INSERT INTO test.aliens ([id],[name]) VALUES(420,'Mork')"
224230
@identity_insert_sql_non_dbo_unordered = "INSERT INTO [test].[aliens] ([name],[id]) VALUES('Mork',420)"
@@ -235,6 +241,10 @@ class AdapterTestSQLServer < ActiveRecord::TestCase
235241
assert_equal "[funny_jokes]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_unquoted_sp)
236242
assert_equal "[funny_jokes]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_unordered_sp)
237243

244+
assert_equal "[ships]", connection.send(:query_requires_identity_insert?, @identity_merge_sql)
245+
assert_equal "[ships]", connection.send(:query_requires_identity_insert?, @identity_merge_sql_unquoted)
246+
assert_equal "[ships]", connection.send(:query_requires_identity_insert?, @identity_merge_sql_unordered)
247+
238248
assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo)
239249
assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo_unquoted)
240250
assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo_unordered)
@@ -244,7 +254,7 @@ class AdapterTestSQLServer < ActiveRecord::TestCase
244254
end
245255

246256
it "return false to #query_requires_identity_insert? for normal SQL" do
247-
[basic_insert_sql, basic_update_sql, basic_select_sql].each do |sql|
257+
[basic_insert_sql, basic_merge_sql, basic_update_sql, basic_select_sql].each do |sql|
248258
assert !connection.send(:query_requires_identity_insert?, sql), "SQL was #{sql}"
249259
end
250260
end

test/cases/schema_test_sqlserver.rb

+18
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,24 @@ class SchemaTestSQLServer < ActiveRecord::TestCase
102102
end
103103
end
104104

105+
describe "MERGE statements" do
106+
it do
107+
assert_equal "[dashboards]", connection.send(:get_raw_table_name, "MERGE INTO [dashboards] AS target")
108+
end
109+
110+
it do
111+
assert_equal "lock_without_defaults", connection.send(:get_raw_table_name, "MERGE INTO lock_without_defaults AS target")
112+
end
113+
114+
it do
115+
assert_equal "[WITH - SPACES]", connection.send(:get_raw_table_name, "MERGE INTO [WITH - SPACES] AS target")
116+
end
117+
118+
it do
119+
assert_equal "[with].[select notation]", connection.send(:get_raw_table_name, "MERGE INTO [with].[select notation] AS target")
120+
end
121+
end
122+
105123
describe "CREATE VIEW statements" do
106124
it do
107125
assert_equal "test_table_as", connection.send(:get_raw_table_name, "CREATE VIEW test_views ( test_table_a_id, test_table_b_id ) AS SELECT test_table_as.id as test_table_a_id, test_table_bs.id as test_table_b_id FROM (test_table_as with(nolock) LEFT JOIN test_table_bs with(nolock) ON (test_table_as.id = test_table_bs.test_table_a_id))")

0 commit comments

Comments
 (0)