Fix multiple nil identity columns for merge insert

[aidanharan]

Handle merge inserts that contain multiple nil identity columns. Eg:

Book.upsert_all [
  { id: nil, name: "New edition 1" },
  { id: nil, name: "New edition 2" },
]

Previously the SQL generated would have been:

MERGE INTO [books] WITH (updlock, holdlock) AS target
using (SELECT *
       FROM   (SELECT [id],
                      [name],
                      [created_at],
                      [updated_at],
                      [updated_on],
                      Row_number()
                        OVER (
                          partition BY [id]
                          ORDER BY [id] DESC ) AS rn_0
               FROM   (VALUES 
                             (Ident_current(N'[books]') + Ident_incr(N'[books]'),
                      N'New edition 2',
                      CURRENT_TIMESTAMP,
                      CURRENT_TIMESTAMP,
                      CURRENT_TIMESTAMP),
                              (Ident_current(N'[books]') + Ident_incr(N'[books]'),
                      N'New edition 1',
                      CURRENT_TIMESTAMP,
                      CURRENT_TIMESTAMP,
                      CURRENT_TIMESTAMP)) AS t1 ([id], [name], [created_at],
                      [updated_at],
                      [updated_on])) AS ranked_source
       WHERE  rn_0 = 1) AS source
ON ( target.[id] = source.[id] )
WHEN matched THEN
  UPDATE SET target.[updated_at] = CASE
                                     WHEN (( source.[name] = target.[name]
                                              OR ( source.[name] IS NULL
                                                   AND target.[name] IS NULL ) )
                                          ) THEN
                                     target.[updated_at]
                                     ELSE CURRENT_TIMESTAMP
                                   END,
             target.[updated_on] = CASE
                                     WHEN (( source.[name] = target.[name]
                                              OR ( source.[name] IS NULL
                                                   AND target.[name] IS NULL ) )
                                          ) THEN
                                     target.[updated_on]
                                     ELSE CURRENT_TIMESTAMP
                                   END,
             target.[name] = source.[name]
WHEN NOT matched BY target THEN
  INSERT ([id],
          [name],
          [created_at],
          [updated_at],
          [updated_on])
  VALUES (source.[id],
          source.[name],
          source.[created_at],
          source.[updated_at],
          source.[updated_on])
output inserted.[id]; 

When this is run only the book with title "New edition 1" is created in the table. The reason is that "New edition 2" is first inserted but then "New edition 1" is updated over "New edition 2" since it matches ON ( target.[id] = source.[id] ).

The fix changes the inserted ID columns to the following so that they don't match and both records get inserted:

• Ident_current(N'[books]') + (Ident_incr(N'[books]') * 1)
• Ident_current(N'[books]') + (Ident_incr(N'[books]') * 2)

This fixes the following tests which were added by rails/rails#54962

• InsertAllTest#test_insert_all_implicitly_sets_primary_keys_when_nil
• InsertAllTest#test_upsert_all_implicitly_sets_primary_keys_when_nil

[andyundso]

just curious: is it possible under MSSQL that you have an identity column that is not a primary key? or is identity column just another word for "column with autogenerated identifier"?

[aidanharan]

Had to look that up. Looks like you can have a primary key that is not an identity column and an identity column that is not a primary key.

CREATE TABLE ExampleTable (
    PrimaryKeyCol INT NOT NULL PRIMARY KEY,
    IdentityColumn INT IDENTITY(1,1) NOT NULL,
    SomeOtherCol NVARCHAR(100) NULL
);

[andyundso]

shouldn't this be enough?

Suggested change

	Arel.sql("IDENT_CURRENT(#{table_name}) + (IDENT_INCR(#{table_name}) * #{identity_index})")
	Arel.sql("IDENT_CURRENT(#{table_name}) + (IDENT_INCR(#{table_name}) + #{identity_index})")

but I am not sure. I assume MSSQL uses the identifier generated here and does not overwrite them, correct? if it does overwrite it, then ignore this comment. if it does not overwrite it, then I would prefer the +, as it would generate much smaller values.

[aidanharan]

The values given by "IDENT_CURRENT(#{table_name}) + (IDENT_INCR(#{table_name}) * #{identity_index})" are the IDs used when the record is created. I used multiplication as IDENT_INCR is the step value (https://learn.microsoft.com/en-us/sql/t-sql/functions/ident-incr-transact-sql?view=sql-server-ver16) that should be used for generating the ID.

So if IDENT_CURRENT('books) is 100 and IDENT_INCR('books') is 2 then the next IDs generated should be 102, 104, 106, etc. To generate this sequence we need to use multiplication. If we used addition then the IDs generated would be 102, 103, 104, etc.