Skip to content

Commit

Permalink
Fix insertion of records to non-default schema table using raw SQL (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
aidanharan authored Apr 24, 2023
1 parent 98e57a6 commit 28c38ea
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 16 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#### Fixed

- [#1052](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1052) Ignore casing of VALUES clause when inserting records using SQL
- [#1053](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1053) Fix insertion of records to non-default schema table using raw SQL

## v7.0.2.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ def query_requires_identity_insert?(sql)
raw_table_name = get_raw_table_name(sql)
id_column = identity_columns(raw_table_name).first

id_column && sql =~ /^\s*(INSERT|EXEC sp_executesql N'INSERT)[^(]+\([^)]*\b(#{id_column.name})\b,?[^)]*\)/i ? SQLServer::Utils.extract_identifiers(raw_table_name).object : false
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
end

def insert_sql?(sql)
Expand Down
2 changes: 1 addition & 1 deletion lib/active_record/connection_adapters/sqlserver/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module Utils
QUOTED_STRING_PREFIX = "N"

# Value object to return identifiers from SQL Server names http://bit.ly/1CZ3EiL
# Inspiried from Rails PostgreSQL::Name adapter object in their own Utils.
# Inspired from Rails PostgreSQL::Name adapter object in their own Utils.
#
class Name
SEPARATOR = "."
Expand Down
40 changes: 32 additions & 8 deletions test/cases/adapter_test_sqlserver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,17 +193,31 @@ class AdapterTestSQLServer < ActiveRecord::TestCase
@identity_insert_sql_unquoted = "INSERT INTO funny_jokes (id, name) VALUES(420, 'Knock knock')"
@identity_insert_sql_unordered = "INSERT INTO [funny_jokes] ([name],[id]) VALUES('Knock knock',420)"
@identity_insert_sql_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'"
@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'"
@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'"
@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"

@identity_insert_sql_non_dbo = "INSERT INTO [test].[aliens] ([id],[name]) VALUES(420,'Mork')"
@identity_insert_sql_non_dbo_unquoted = "INSERT INTO test.aliens ([id],[name]) VALUES(420,'Mork')"
@identity_insert_sql_non_dbo_unordered = "INSERT INTO [test].[aliens] ([name],[id]) VALUES('Mork',420)"
@identity_insert_sql_non_dbo_sp = "EXEC sp_executesql N'INSERT INTO [test].[aliens] ([id],[name]) VALUES (@0, @1)', N'@0 int, @1 nvarchar(255)', @0 = 420, @1 = N'Mork'"
@identity_insert_sql_non_dbo_unquoted_sp = "EXEC sp_executesql N'INSERT INTO test.aliens (id, name) VALUES (@0, @1)', N'@0 int, @1 nvarchar(255)', @0 = 420, @1 = N'Mork'"
@identity_insert_sql_non_dbo_unordered_sp = "EXEC sp_executesql N'INSERT INTO [test].[aliens] ([name],[id]) VALUES (@0, @1)', N'@0 nvarchar(255), @1 int', @0 = N'Mork', @1 = 420"
end

it "return unquoted table_name to #query_requires_identity_insert? when INSERT sql contains id column" do
assert_equal "funny_jokes", connection.send(:query_requires_identity_insert?, @identity_insert_sql)
assert_equal "funny_jokes", connection.send(:query_requires_identity_insert?, @identity_insert_sql_unquoted)
assert_equal "funny_jokes", connection.send(:query_requires_identity_insert?, @identity_insert_sql_unordered)
assert_equal "funny_jokes", connection.send(:query_requires_identity_insert?, @identity_insert_sql_sp)
assert_equal "funny_jokes", connection.send(:query_requires_identity_insert?, @identity_insert_sql_unquoted_sp)
assert_equal "funny_jokes", connection.send(:query_requires_identity_insert?, @identity_insert_sql_unordered_sp)
it "return quoted table_name to #query_requires_identity_insert? when INSERT sql contains id column" do
assert_equal "[funny_jokes]", connection.send(:query_requires_identity_insert?, @identity_insert_sql)
assert_equal "[funny_jokes]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_unquoted)
assert_equal "[funny_jokes]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_unordered)
assert_equal "[funny_jokes]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_sp)
assert_equal "[funny_jokes]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_unquoted_sp)
assert_equal "[funny_jokes]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_unordered_sp)

assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo)
assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo_unquoted)
assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo_unordered)
assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo_sp)
assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo_unquoted_sp)
assert_equal "[test].[aliens]", connection.send(:query_requires_identity_insert?, @identity_insert_sql_non_dbo_unordered_sp)
end

it "return false to #query_requires_identity_insert? for normal SQL" do
Expand Down Expand Up @@ -533,6 +547,16 @@ def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes
end
end

describe 'table is in non-dbo schema' do
it "records can be created successfully" do
Alien.create!(name: 'Trisolarans')
end

it 'records can be inserted using SQL' do
Alien.connection.exec_insert("insert into [test].[aliens] (id, name) VALUES(1, 'Trisolarans'), (2, 'Xenomorph')")
end
end

describe "exec_insert" do
it 'values clause should be case-insensitive' do
assert_difference("Post.count", 4) do
Expand Down
4 changes: 0 additions & 4 deletions test/cases/schema_test_sqlserver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,5 @@ class SchemaTestSQLServer < ActiveRecord::TestCase
assert_equal 255, columns.find { |c| c.name == "n_name" }.limit
assert_equal 1000, columns.find { |c| c.name == "n_description" }.limit
end

it "creates new record" do
Alien.create!(name: 'Trisolarans')
end
end
end
4 changes: 2 additions & 2 deletions test/cases/utils_test_sqlserver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class UtilsTestSQLServer < ActiveRecord::TestCase
_(extract_identifiers(" ").object).must_be_nil
end

it "has a #quoted that returns a fully quoted name with all identifiers as orginially passed in" do
it "has a #quoted that returns a fully quoted name with all identifiers as originally passed in" do
_(extract_identifiers("object").quoted).must_equal "[object]"
_(extract_identifiers("server.database..object").quoted).must_equal "[server].[database]..[object]"
_(extract_identifiers("[server]...[object]").quoted).must_equal "[server]...[object]"
Expand All @@ -92,7 +92,7 @@ class UtilsTestSQLServer < ActiveRecord::TestCase
_(extract_identifiers("[obj.name].[foo]").quoted).must_equal "[obj.name].[foo]"
end

it "should indicate if a name is fully qualitified" do
it "should indicate if a name is fully qualified" do
_(extract_identifiers("object").fully_qualified?).must_equal false
_(extract_identifiers("schema.object").fully_qualified?).must_equal false
_(extract_identifiers("database.schema.object").fully_qualified?).must_equal false
Expand Down
1 change: 1 addition & 0 deletions test/schema/sqlserver_specific_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@
execute "IF EXISTS(SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'aliens' and TABLE_SCHEMA = 'test') DROP TABLE test.aliens"
execute <<-TABLE_IN_OTHER_SCHEMA_USED_BY_MODEL
CREATE TABLE test.aliens(
id int IDENTITY NOT NULL primary key,
name varchar(255)
)
TABLE_IN_OTHER_SCHEMA_USED_BY_MODEL
Expand Down

0 comments on commit 28c38ea

Please sign in to comment.