From f87dbd61722888cc6ed019917e381ebd3ae07092 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Tue, 16 Jul 2024 17:22:53 +0100 Subject: [PATCH 01/12] Parse table names with spaces correctly --- .../connection_adapters/sqlserver/schema_statements.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index afcc0874b..300f521aa 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -674,7 +674,7 @@ def get_table_name(sql) # Parses the raw table name that is used in the SQL. Table name could include database/schema/etc. def get_raw_table_name(sql) case sql - when /^\s*(INSERT|EXEC sp_executesql N'INSERT)(\s+INTO)?\s+([^\(\s]+)\s*|^\s*update\s+([^\(\s]+)\s*/i + when /^\s*(INSERT|EXEC sp_executesql N'INSERT)(\s+INTO)?\s+([^\(]+)\s*|^\s*update\s+([^\(\s]+)\s*/i Regexp.last_match[3] || Regexp.last_match[4] when /FROM\s+([^\(\s]+)\s*/i Regexp.last_match[1] From af4a195985f34eb56975aff3db72da085803f435 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Wed, 17 Jul 2024 10:27:10 +0100 Subject: [PATCH 02/12] Debug --- .../connection_adapters/sqlserver/database_statements.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index 308b7050e..84905a142 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -32,6 +32,9 @@ def raw_execute(sql, name, async: false, allow_retry: false, materialize_transac def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) result = nil + + puts "******** SQL: #{sql}" + sql = transform_query(sql) check_if_write_query(sql) @@ -395,6 +398,8 @@ def exclude_output_inserted_id_sql_type(pk, exclude_output_inserted) def query_requires_identity_insert?(sql) return false unless insert_sql?(sql) + # binding.pry if $DEBUG + raw_table_name = get_raw_table_name(sql) id_column = identity_columns(raw_table_name).first From 9e31bd90f12e29dc01a7ac49ca5e588f2dcfd87d Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Wed, 17 Jul 2024 10:32:23 +0100 Subject: [PATCH 03/12] Debug --- .../connection_adapters/sqlserver/schema_statements.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index 300f521aa..767226c00 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -673,12 +673,14 @@ def get_table_name(sql) # Parses the raw table name that is used in the SQL. Table name could include database/schema/etc. def get_raw_table_name(sql) + puts "get_raw_table_name: SQL: #{sql}" + case sql when /^\s*(INSERT|EXEC sp_executesql N'INSERT)(\s+INTO)?\s+([^\(]+)\s*|^\s*update\s+([^\(\s]+)\s*/i Regexp.last_match[3] || Regexp.last_match[4] when /FROM\s+([^\(\s]+)\s*/i Regexp.last_match[1] - end + end.strip end def default_constraint_name(table_name, column_name) From f0d38ad5767cd8193940150e7ca1770c987f9b71 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Wed, 17 Jul 2024 10:40:38 +0100 Subject: [PATCH 04/12] Remove debug --- .../connection_adapters/sqlserver/database_statements.rb | 3 --- .../connection_adapters/sqlserver/schema_statements.rb | 2 -- 2 files changed, 5 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index 84905a142..c4d247dd0 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -32,9 +32,6 @@ def raw_execute(sql, name, async: false, allow_retry: false, materialize_transac def internal_exec_query(sql, name = "SQL", binds = [], prepare: false, async: false) result = nil - - puts "******** SQL: #{sql}" - sql = transform_query(sql) check_if_write_query(sql) diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index 767226c00..8ed60bb08 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -673,8 +673,6 @@ def get_table_name(sql) # Parses the raw table name that is used in the SQL. Table name could include database/schema/etc. def get_raw_table_name(sql) - puts "get_raw_table_name: SQL: #{sql}" - case sql when /^\s*(INSERT|EXEC sp_executesql N'INSERT)(\s+INTO)?\s+([^\(]+)\s*|^\s*update\s+([^\(\s]+)\s*/i Regexp.last_match[3] || Regexp.last_match[4] From 22c5ef82aa1c07151bdfd56af65939056a31009c Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Wed, 17 Jul 2024 10:55:12 +0100 Subject: [PATCH 05/12] Debug --- .../connection_adapters/sqlserver/database_statements.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index c4d247dd0..b23478910 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -401,6 +401,15 @@ def query_requires_identity_insert?(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).quoted : false + + rescue StandardError => e + + puts "*" * 100 + puts "sql: #{sql}" + puts "raw_table_name: #{raw_table_name}" + puts "*" * 100 + + raise e end def insert_sql?(sql) From 5624f8f0be04e82bce4f11efb5776146a3f190ec Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Wed, 17 Jul 2024 13:41:10 +0100 Subject: [PATCH 06/12] Update schema_statements.rb --- .../connection_adapters/sqlserver/schema_statements.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index 8ed60bb08..d5587b00a 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -673,10 +673,12 @@ def get_table_name(sql) # Parses the raw table name that is used in the SQL. Table name could include database/schema/etc. def get_raw_table_name(sql) + + case sql - when /^\s*(INSERT|EXEC sp_executesql N'INSERT)(\s+INTO)?\s+([^\(]+)\s*|^\s*update\s+([^\(\s]+)\s*/i + when /^\s*(INSERT|EXEC sp_executesql N'INSERT)(\s+INTO)?\s+(\[[^\(\]]+\])\s*|^\s*update\s+([^\(\s]+)\s*/i Regexp.last_match[3] || Regexp.last_match[4] - when /FROM\s+([^\(\s]+)\s*/i + when /FROM\s+((\[[^\(\]]+\])|[^\(\s]+)\s*/i Regexp.last_match[1] end.strip end From 0a76dd356ac4c49f783085d5fcda382151468ea3 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Wed, 17 Jul 2024 18:28:59 +0100 Subject: [PATCH 07/12] Don't try to get table name in single regex --- .../sqlserver/schema_statements.rb | 23 +++++++-- test/cases/schema_test_sqlserver.rb | 48 ++++++++++++++++++- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index d5587b00a..44672f8cb 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -674,13 +674,26 @@ def get_table_name(sql) # Parses the raw table name that is used in the SQL. Table name could include database/schema/etc. def get_raw_table_name(sql) + s = sql.gsub(/^\s*EXEC sp_executesql N'/i, "") - case sql - when /^\s*(INSERT|EXEC sp_executesql N'INSERT)(\s+INTO)?\s+(\[[^\(\]]+\])\s*|^\s*update\s+([^\(\s]+)\s*/i - Regexp.last_match[3] || Regexp.last_match[4] - when /FROM\s+((\[[^\(\]]+\])|[^\(\s]+)\s*/i - Regexp.last_match[1] + # binding.pry + + if s.match?(/^\s*INSERT INTO.*/i) + s = s.split(/INSERT INTO/i)[1].split(/OUTPUT INSERTED/i)[0].split(/(DEFAULT)?\s+VALUES/i)[0] + + s.match(/\s*([^(]*)/i)[0] + else + s.match(/FROM\s+((\[[^\(\]]+\])|[^\(\s]+)\s*/i)[1] end.strip + + # table_name + + # case sql + # when /^\s*(INSERT|EXEC sp_executesql N'INSERT)(\s+INTO)?\s+(\[[^\(\]]+\])\s*|^\s*update\s+([^\(\s]+)\s*/i + # Regexp.last_match[3] || Regexp.last_match[4] + # when /FROM\s+((\[[^\(\]]+\])|[^\(\s]+)\s*/i + # Regexp.last_match[1] + # end.strip end def default_constraint_name(table_name, column_name) diff --git a/test/cases/schema_test_sqlserver.rb b/test/cases/schema_test_sqlserver.rb index fc119fe27..1751af622 100644 --- a/test/cases/schema_test_sqlserver.rb +++ b/test/cases/schema_test_sqlserver.rb @@ -39,7 +39,7 @@ class SchemaTestSQLServer < ActiveRecord::TestCase assert_equal 1, columns.select { |c| c.is_identity? }.size end - it "return correct varchar and nvarchar column limit length when table is in non dbo schema" do + it "return correct varchar and nvarchar column limit length when table is in non-dbo schema" do columns = connection.columns("test.sst_schema_columns") assert_equal 255, columns.find { |c| c.name == "name" }.limit @@ -48,4 +48,50 @@ class SchemaTestSQLServer < ActiveRecord::TestCase assert_equal 1000, columns.find { |c| c.name == "n_description" }.limit end end + + describe "parsing table name from raw SQL" do + describe 'SELECT statements' do + it do + assert_equal "[sst_schema_columns]", connection.send(:get_raw_table_name, "SELECT [sst_schema_columns].[id] FROM [sst_schema_columns]") + end + + it do + assert_equal "sst_schema_columns", connection.send(:get_raw_table_name, "SELECT [sst_schema_columns].[id] FROM sst_schema_columns") + end + + it do + assert_equal "[WITH - SPACES]", connection.send(:get_raw_table_name, "SELECT id FROM [WITH - SPACES]") + end + + it do + assert_equal "[WITH - SPACES$DOLLAR]", connection.send(:get_raw_table_name, "SELECT id FROM [WITH - SPACES$DOLLAR]") + end + end + + describe 'INSERT statements' do + it do + assert_equal "[dashboards]", connection.send(:get_raw_table_name, "INSERT INTO [dashboards] DEFAULT VALUES; SELECT CAST(SCOPE_IDENTITY() AS bigint) AS Ident") + end + + it do + assert_equal "lock_without_defaults", connection.send(:get_raw_table_name, "INSERT INTO lock_without_defaults(title) VALUES('title1')") + end + + it do + assert_equal "json_data_type", connection.send(:get_raw_table_name, "insert into json_data_type (payload) VALUES ('null')") + end + + it do + assert_equal "[auto_increments]", connection.send(:get_raw_table_name, "INSERT INTO [auto_increments] OUTPUT INSERTED.[id] DEFAULT VALUES") + end + + it do + assert_equal "[WITH - SPACES]", connection.send(:get_raw_table_name, "EXEC sp_executesql N'INSERT INTO [WITH - SPACES] ([external_id]) OUTPUT INSERTED.[id] VALUES (@0)', N'@0 bigint', @0 = 10") + end + + it do + assert_equal "[test].[aliens]", connection.send(:get_raw_table_name, "EXEC sp_executesql N'INSERT INTO [test].[aliens] ([name]) OUTPUT INSERTED.[id] VALUES (@0)', N'@0 varchar(255)', @0 = 'Trisolarans'") + end + end + end end From edcf5e50db25d40d6ba6035de1584844aaf0b412 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Wed, 17 Jul 2024 18:56:44 +0100 Subject: [PATCH 08/12] Update schema_statements.rb --- .../connection_adapters/sqlserver/schema_statements.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index 44672f8cb..adf0ecef3 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -682,6 +682,8 @@ def get_raw_table_name(sql) s = s.split(/INSERT INTO/i)[1].split(/OUTPUT INSERTED/i)[0].split(/(DEFAULT)?\s+VALUES/i)[0] s.match(/\s*([^(]*)/i)[0] + elsif s.match?(/^\s*UPDATE\s+.*/i) + s.match(/UPDATE\s+([^\(\s]+)\s*/i)[1] else s.match(/FROM\s+((\[[^\(\]]+\])|[^\(\s]+)\s*/i)[1] end.strip From 44d115fbfc5832816dad07363cc3f7b3ce0a0c97 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Wed, 17 Jul 2024 20:19:02 +0100 Subject: [PATCH 09/12] Cleanup --- .../sqlserver/database_statements.rb | 9 --------- .../sqlserver/schema_statements.rb | 19 ++++--------------- 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index b23478910..c4d247dd0 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -401,15 +401,6 @@ def query_requires_identity_insert?(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).quoted : false - - rescue StandardError => e - - puts "*" * 100 - puts "sql: #{sql}" - puts "raw_table_name: #{raw_table_name}" - puts "*" * 100 - - raise e end def insert_sql?(sql) diff --git a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb index adf0ecef3..636bcfc7c 100644 --- a/lib/active_record/connection_adapters/sqlserver/schema_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/schema_statements.rb @@ -673,29 +673,18 @@ def get_table_name(sql) # Parses the raw table name that is used in the SQL. Table name could include database/schema/etc. def get_raw_table_name(sql) - s = sql.gsub(/^\s*EXEC sp_executesql N'/i, "") - # binding.pry - if s.match?(/^\s*INSERT INTO.*/i) - s = s.split(/INSERT INTO/i)[1].split(/OUTPUT INSERTED/i)[0].split(/(DEFAULT)?\s+VALUES/i)[0] - - s.match(/\s*([^(]*)/i)[0] + s.split(/INSERT INTO/i)[1] + .split(/OUTPUT INSERTED/i)[0] + .split(/(DEFAULT)?\s+VALUES/i)[0] + .match(/\s*([^(]*)/i)[0] elsif s.match?(/^\s*UPDATE\s+.*/i) s.match(/UPDATE\s+([^\(\s]+)\s*/i)[1] else s.match(/FROM\s+((\[[^\(\]]+\])|[^\(\s]+)\s*/i)[1] end.strip - - # table_name - - # case sql - # when /^\s*(INSERT|EXEC sp_executesql N'INSERT)(\s+INTO)?\s+(\[[^\(\]]+\])\s*|^\s*update\s+([^\(\s]+)\s*/i - # Regexp.last_match[3] || Regexp.last_match[4] - # when /FROM\s+((\[[^\(\]]+\])|[^\(\s]+)\s*/i - # Regexp.last_match[1] - # end.strip end def default_constraint_name(table_name, column_name) From b0f64ef686bc4706e6f51ab3f8fba6d6a5b01dc2 Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Wed, 17 Jul 2024 21:33:21 +0100 Subject: [PATCH 10/12] Update database_statements.rb --- .../connection_adapters/sqlserver/database_statements.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/active_record/connection_adapters/sqlserver/database_statements.rb b/lib/active_record/connection_adapters/sqlserver/database_statements.rb index c4d247dd0..308b7050e 100644 --- a/lib/active_record/connection_adapters/sqlserver/database_statements.rb +++ b/lib/active_record/connection_adapters/sqlserver/database_statements.rb @@ -395,8 +395,6 @@ def exclude_output_inserted_id_sql_type(pk, exclude_output_inserted) def query_requires_identity_insert?(sql) return false unless insert_sql?(sql) - # binding.pry if $DEBUG - raw_table_name = get_raw_table_name(sql) id_column = identity_columns(raw_table_name).first From 6ce7a13b7ff46ba4975e53c66ec9429652d88ccf Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 18 Jul 2024 15:37:39 +0100 Subject: [PATCH 11/12] Added test --- test/cases/adapter_test_sqlserver.rb | 16 ++++++++++++++-- test/models/sqlserver/table_with_spaces.rb | 5 +++++ test/schema/sqlserver_specific_schema.rb | 4 ++++ 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 test/models/sqlserver/table_with_spaces.rb diff --git a/test/cases/adapter_test_sqlserver.rb b/test/cases/adapter_test_sqlserver.rb index 34394732b..166920e4f 100644 --- a/test/cases/adapter_test_sqlserver.rb +++ b/test/cases/adapter_test_sqlserver.rb @@ -550,11 +550,23 @@ def test_doesnt_error_when_a_select_query_is_called_while_preventing_writes describe 'table is in non-dbo schema' do it "records can be created successfully" do - Alien.create!(name: 'Trisolarans') + assert_difference("Alien.count", 1) do + Alien.create!(name: 'Trisolarans') + end end it 'records can be inserted using SQL' do - Alien.connection.exec_insert("insert into [test].[aliens] (id, name) VALUES(1, 'Trisolarans'), (2, 'Xenomorph')") + assert_difference("Alien.count", 2) do + Alien.connection.exec_insert("insert into [test].[aliens] (id, name) VALUES(1, 'Trisolarans'), (2, 'Xenomorph')") + end + end + end + + describe 'table names contains spaces' do + it 'records can be created successfully' do + assert_difference("TableWithSpaces.count", 1) do + TableWithSpaces.create!(name: 'Bob') + end end end diff --git a/test/models/sqlserver/table_with_spaces.rb b/test/models/sqlserver/table_with_spaces.rb new file mode 100644 index 000000000..d5f07ec4a --- /dev/null +++ b/test/models/sqlserver/table_with_spaces.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class TableWithSpaces < ActiveRecord::Base + self.table_name = "A Table With Spaces" +end diff --git a/test/schema/sqlserver_specific_schema.rb b/test/schema/sqlserver_specific_schema.rb index 1bef7d0e1..b11cd6fa9 100644 --- a/test/schema/sqlserver_specific_schema.rb +++ b/test/schema/sqlserver_specific_schema.rb @@ -151,6 +151,10 @@ SELECT GETUTCDATE() utcdate SQL + create_table 'A Table With Spaces', force: true do |t| + t.string :name + end + # Constraints create_table(:sst_has_fks, force: true) do |t| From 66188d3255b86f9fb8df8f8e3a354dd0a8c4b75e Mon Sep 17 00:00:00 2001 From: Aidan Haran Date: Thu, 18 Jul 2024 15:39:34 +0100 Subject: [PATCH 12/12] Update CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2226fab10..1c7cd4b84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ #### Added -- [#1201](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1201) Support non-dbo schemas in schema dumper. +- [#1201](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1201) Support non-dbo schemas in schema dumper +- [#1206](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/1206) Support table names containing spaces ## v7.1.4