Improve renameColumn generated SQL on mssql

[mide25]

Overview

The existing rename column logic on mssql does not specify it is a column that is being renamed. This works fine as long as there are not objects of other types in the database with the same name.

The fix updates the SQL to explicitly say way are updating a column to avoid the abiguity.

Fix details

Mssql fix to follow syntax as per msdn:
sp_rename [ objname = ] 'object_name' , [ newname = ] 'new_name'

[ , [ @objtype = ] 'object_type' ]

https://docs.microsoft.com/en-us/sql/relational-databases/system-stored-procedures/sp-rename-transact-sql?view=sql-server-2017

Fixes CORE-3276

[ro-rah]

@mide25

Thanks for your pull request!

Here’s what happens next:

A member of the Liquibase team will take a look at your contribution and may suggest:

• Changes based on guidelines for PR submission
• Additional tests (unit and/or integration)
• Provide other feedback, like requesting changes.

The PR will be prioritized according to our internal development and testing capacity.

We’ll let you know when it’s ready to move to the next step or if any changes are needed.

[ro-rah]

@mide25 would you be willing to add unit or integration tests for this change?

Till then I will size at a Medium.

[liquibot]

➤ Erzsebet Carmean commented:

I'm flagging this for internal discussion of repro steps and type of testing. This looks like it could fall into a unit test rather than a full e2e functional test.

-erz (@ XDelphiGrl)

CC : [~accountid:557058:c8f0b24d-f02f-431c-8842-fd9c929dfbf2]
CC: [~accountid:5d937e71f5d3c10d8bfd351d]

[nvoxland]

It is a slightly better version of the existing sql. The existing integration tests should catch if it's a problem

[liquibot]

➤ Erzsebet Carmean commented:

Thank you for the response, Nathan! -erz (@ XDelphiGrl)

CC : [~accountid:557058:c8f0b24d-f02f-431c-8842-fd9c929dfbf2]

[michaelplavnik]

Please include this PR into closest release, as it fixes another severe issue, rename of the colum on system versioned table. Without explicitely specifying 'COLUMN' object type parameter, rename will affect primary table, but will not touch version table. Today it causes liquibase users to put verbose SQL server specific SQL to workaround.

[molivasdat]

HI @michaelplavnik Thanks for letting us know that this is still a priority. We will look to add it soon. It did not make 4.3 or 4.3.1.

[kataggart]

@nvoxland looks like back in the day you reviewed this and approved it, but given it's age It likely needs a re-review and a re-build. Thanks!

[github-actions]

Unit Test Results

  4 608 files  ±0    4 608 suites  ±0   31m 41s ⏱️ - 4m 46s
  4 589 tests ±0    4 375 ✔️ ±0     214 💤 ±0  0 ❌ ±0 
54 240 runs  ±0  49 232 ✔️ ±0  5 008 💤 ±0  0 ❌ ±0 

Results for commit 7b2c9f5. ± Comparison against base commit bcd2d53.

♻️ This comment has been updated with latest results.

[nvoxland]

Code review and test results:

Change makes sense, it adds an extra and helpful argument to the rename call to better handle the defined bug.

Things to be aware of:

• Existing tests call renameColumn on mssql and continue to pass

Things to worry about:

• Nothing

[XDelphiGrl]

• Targeted fix for renaming a SQL Server column.
• New unit test added.
• No additional testing required.

APPROVED

Passing Functional Tests
Passing Test Harness Execution