Skip to content

Fix composite primary key with different data type with triggers #1164

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ adapter.exclude_output_inserted_table_names['my_table_name'] = true

# Explicitly set the data type for the temporary key table.
adapter.exclude_output_inserted_table_names['my_uuid_table_name'] = 'uniqueidentifier'


# Explicitly set data types when data type is different for composite primary keys.
adapter.exclude_output_inserted_table_names['my_composite_pk_table_name'] = { pk_col_one: "uniqueidentifier", pk_col_two: "int" }
```


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,17 @@ def sql_for_insert(sql, pk, binds, returning)
exclude_output_inserted = exclude_output_inserted_table_name?(table_name, sql)

if exclude_output_inserted
quoted_pk = Array(pk).map { |subkey| SQLServer::Utils.extract_identifiers(subkey).quoted }
pk_and_types = Array(pk).map do |subkey|
{
quoted: SQLServer::Utils.extract_identifiers(subkey).quoted,
id_sql_type: exclude_output_inserted_id_sql_type(subkey, exclude_output_inserted)
}
end

id_sql_type = exclude_output_inserted.is_a?(TrueClass) ? "bigint" : exclude_output_inserted
<<~SQL.squish
DECLARE @ssaIdInsertTable table (#{quoted_pk.map { |subkey| "#{subkey} #{id_sql_type}"}.join(", ") });
#{sql.dup.insert sql.index(/ (DEFAULT )?VALUES/i), " OUTPUT #{ quoted_pk.map { |subkey| "INSERTED.#{subkey}" }.join(", ") } INTO @ssaIdInsertTable"}
SELECT #{quoted_pk.map {|subkey| "CAST(#{subkey} AS #{id_sql_type}) #{subkey}"}.join(", ")} FROM @ssaIdInsertTable
DECLARE @ssaIdInsertTable table (#{pk_and_types.map { |pk_and_type| "#{pk_and_type[:quoted]} #{pk_and_type[:id_sql_type]}"}.join(", ") });
#{sql.dup.insert sql.index(/ (DEFAULT )?VALUES/i), " OUTPUT #{ pk_and_types.map { |pk_and_type| "INSERTED.#{pk_and_type[:quoted]}" }.join(", ") } INTO @ssaIdInsertTable"}
SELECT #{pk_and_types.map {|pk_and_type| "CAST(#{pk_and_type[:quoted]} AS #{pk_and_type[:id_sql_type]}) #{pk_and_type[:quoted]}"}.join(", ")} FROM @ssaIdInsertTable
SQL
else
returning_columns = returning || Array(pk)
Expand Down Expand Up @@ -385,6 +389,12 @@ def exclude_output_inserted_table_name?(table_name, sql)
self.class.exclude_output_inserted_table_names[table_name]
end

def exclude_output_inserted_id_sql_type(pk, exclude_output_inserted)
return "bigint" if exclude_output_inserted.is_a?(TrueClass)
return exclude_output_inserted[pk.to_sym] if exclude_output_inserted.is_a?(Hash)
exclude_output_inserted
end

def query_requires_identity_insert?(sql)
return false unless insert_sql?(sql)

Expand Down
10 changes: 10 additions & 0 deletions test/cases/trigger_test_sqlserver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,14 @@ class SQLServerTriggerTest < ActiveRecord::TestCase
_(obj.pk_col_two).must_equal 42
_(obj.pk_col_one.to_s).must_equal SSTestTriggerHistory.first.id_source
end

it "can insert into a table with composite pk with different data type with output inserted - with a hash setting for table name" do
exclude_output_inserted_table_names["sst_table_with_composite_pk_trigger_with_different_data_type"] = { pk_col_one: "uniqueidentifier", pk_col_two: "int" }
assert SSTestTriggerHistory.all.empty?
obj = SSTestTriggerCompositePkWithDefferentDataType.create! pk_col_two: 123, event_name: "test trigger"
_(obj.event_name).must_equal "test trigger"
_(obj.pk_col_one).must_be :present?
_(obj.pk_col_two).must_equal 123
_(obj.pk_col_one.to_s).must_equal SSTestTriggerHistory.first.id_source
end
end
4 changes: 4 additions & 0 deletions test/models/sqlserver/trigger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ class SSTestTriggerUuid < ActiveRecord::Base
class SSTestTriggerCompositePk < ActiveRecord::Base
self.table_name = "sst_table_with_composite_pk_trigger"
end

class SSTestTriggerCompositePkWithDefferentDataType < ActiveRecord::Base
self.table_name = "sst_table_with_composite_pk_trigger_with_different_data_type"
end
17 changes: 17 additions & 0 deletions test/schema/sqlserver_specific_schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,23 @@
SELECT pk_col_one AS id_source, event_name FROM INSERTED
SQL

execute "IF EXISTS(SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'sst_table_with_composite_pk_trigger_with_different_data_type') DROP TABLE sst_table_with_composite_pk_trigger_with_different_data_type"
execute <<-SQL
CREATE TABLE sst_table_with_composite_pk_trigger_with_different_data_type(
pk_col_one uniqueidentifier DEFAULT NEWID(),
pk_col_two int NOT NULL,
event_name nvarchar(255),
CONSTRAINT PK_sst_table_with_composite_pk_trigger_with_different_data_type PRIMARY KEY (pk_col_one, pk_col_two)
)
SQL
execute <<-SQL
CREATE TRIGGER sst_table_with_composite_pk_trigger_with_different_data_type_t ON sst_table_with_composite_pk_trigger_with_different_data_type
FOR INSERT
AS
INSERT INTO sst_table_with_trigger_history (id_source, event_name)
SELECT pk_col_one AS id_source, event_name FROM INSERTED
SQL

# Another schema.

create_table :sst_schema_columns, force: true do |t|
Expand Down
Loading