Migration script fails to set DB rev, modifies SP instead

ismeisme Posts: 119
edited February 17, 2017 9:57AM in SQL Source Control Previous Versions
Hey guys,

Today we created and tested a custom migration script for a complex schema change.

The migration script failed to set the revision number extended property, which caused an error in a later comparison. I've already raised a private bug report about that.

The ‘Commit Changes’ tab reveals that the code to set the extended property became part of a stored procedure instead of being executed against the target.

The code was added to the last stored procedure in the update script.

Looks like the SQL Source Control didn't add a batch separator between the stored procedure definition and the non-schema statements.

Is someone from Red Gate able to reproduce this?

Can you make sure that SSC always includes a batch separator between the schema definition and the non-schema statements?

Here’s what the ‘Commit Changes’ tab shows:
CREATE PROCEDURE [Customer].[uspGetPurchases]   
AS
BEGIN  
       SET NOCOUNT ON;

       SELECT 
                ProductId = p.PK_ProductID,
                ProductName = p.ProductName,
                ProductType = p.ProductType
       FROM Customer.tbProduct p


       SELECT 
				PurchaseId = cp.PurchaseId, 	
                APIkey,
                ProductId = cp.FK_ProductID,
                ParameterName = p.ParameterName,
                ParameterValue = pv.ParameterValue
       FROM Customer.tbCustomerProductParameter cpp 
       INNER JOIN Customer.tbCustomerProduct cp ON cpp.FK_CustomerProductID = cp.PK_CustomerProductID
       INNER JOIN Customer.tbParameter p ON cpp.FK_ParameterID = p.PK_ParameterID
       INNER JOIN Customer.tbParameterValue pv ON cpp.FK_ParameterValueID = pv.PK_ParameterValueID
       INNER JOIN Customer.tbCustomer c ON cp.FK_CustomerID = c.PK_CustomerID

END
/*
End of custom script APIServices 20124 to 20130 migrating from version 20124 to version 20130
*/
/*
Start of RedGate SQL Source Control versioning database-level extended properties.
*/
DECLARE @RG_SC_VERSION BIGINT
SET @RG_SC_VERSION = 20130
IF EXISTS (SELECT 1 FROM fn_listextendedproperty(N'SQLSourceControl Database Revision', NULL, NULL, NULL, NULL, NULL, NULL))
  EXEC sp_dropextendedproperty N'SQLSourceControl Database Revision', NULL, NULL, NULL, NULL, NULL, NULL
EXEC sp_addextendedproperty N'SQLSourceControl Database Revision', @RG_SC_VERSION, NULL, NULL, NULL, NULL, NULL, NULL
GO

My SQL Source Control version is 3.5.3.29.
Iain Elder, Skyscanner
· Share on Twitter

Comments

  • ismeisme Posts: 119
    In version control the migration script ends with a batch terminator and a blank line.
    ...
       INNER JOIN Customer.tbCustomer c ON cp.FK_CustomerID = c.PK_CustomerID
    
END
GO

    It looks like SQL Source Control strips the batch terminator before appending the non-schema statements when it prepares the sync script.
    Iain Elder, Skyscanner
    · Share on Twitter
  • James BJames B Posts: 1,124 Silver 4
    Hi Iain,

    I've just had a look at this myself. I do see the same behaviour as you (I think!) - the migration script in the repo has a terminating 'GO' but when we execute the script on a Get Latest, the GO seems to be omitted. Interestingly, in my case, this didn't stop the extended properties from getting updated so I'm not sure that the cause of that is necessarily the same.

    In any case, I'll log a bug for that, although I'm not sure as to the likelihood of it getting resolved. This is because we're re-working the whole migrations function which will work completely differently (see here for details)
    Systems Software Engineer

    Redgate Software

    · Share on Twitter
  • ismeisme Posts: 119
    james.billings wrote:
    in my case, this didn't stop the extended properties from getting updated so I'm not sure that the cause of that is necessarily the same.

    What was the type of the last object in your sync script?

    I can reproduce your behavior if the last object is a table.
    CREATE TABLE x (y INT);

DECLARE @RG_SC_VERSION BIGINT
SET @RG_SC_VERSION = 123
IF EXISTS (SELECT 1 FROM fn_listextendedproperty(N'SQLSourceControl Database Revision', NULL, NULL, NULL, NULL, NULL, NULL))
  EXEC sp_dropextendedproperty N'SQLSourceControl Database Revision', NULL, NULL, NULL, NULL, NULL, NULL
EXEC sp_addextendedproperty N'SQLSourceControl Database Revision', @RG_SC_VERSION, NULL, NULL, NULL, NULL, NULL, NULL
GO

    The intention of this batch is to create table x and set the version property to 123.

    The batch separator is missing, but it behaves as you expect.

    It's not possible for the non-schema statements to be part of the table definition, so SQL Server creates the table then applies the extended property.

    You can see that it worked when you query the database metadata.
    /*------------------------
SELECT name, create_date FROM sys.objects WHERE name = N'x';

SELECT name, value FROM fn_listextendedproperty(N'SQLSourceControl Database Revision', NULL, NULL, NULL, NULL, NULL, NULL)
------------------------*/

name                                create_date
----------------------------------- -----------------------
x                                   2013-12-04 11:41:48.390


name                                value
----------------------------------- ------------------------
SQLSourceControl Database Revision  123

    One table was created, and the extended property was updated. Perfect!

    But there's a problem when the last object is a stored procedure.
    CREATE PROCEDURE y
AS
BEGIN
  SELECT 1;
END;

DECLARE @RG_SC_VERSION BIGINT
SET @RG_SC_VERSION = 456
IF EXISTS (SELECT 1 FROM fn_listextendedproperty(N'SQLSourceControl Database Revision', NULL, NULL, NULL, NULL, NULL, NULL))
  EXEC sp_dropextendedproperty N'SQLSourceControl Database Revision', NULL, NULL, NULL, NULL, NULL, NULL
EXEC sp_addextendedproperty N'SQLSourceControl Database Revision', @RG_SC_VERSION, NULL, NULL, NULL, NULL, NULL, NULL
GO

    This batch looks like it creates stored procedure y and sets the version property to 456. Instead it creates a stored procedure that contains the code to set the property.

    The missing batch separator makes an important difference. A stored procedure definition begins immediately after the AS and continues to the end of the batch. The BEGIN...END is just syntactic sugar; SQL Server ignores it.

    You can see something is wrong when you query the metadata.
    /*------------------------
SELECT name, create_date FROM sys.objects WHERE name = N'y';

SELECT name, value FROM fn_listextendedproperty(N'SQLSourceControl Database Revision', NULL, NULL, NULL, NULL, NULL, NULL)
------------------------*/

name                                create_date
----------------------------------- ------------------------
y                                   2013-12-04 12:08:45.607


name                                value
----------------------------------- ------------------------
SQLSourceControl Database Revision  123

    One procedure was created, but the extended property was not updated. Uh-oh!

    You can see the code we meant to execute is inside the stored procedure when you print its definition.
    /*------------------------
DECLARE @definition NVARCHAR(MAX) = (SELECT [definition] FROM sys.sql_modules WHERE OBJECT_NAME([object_id]) = N'y');
PRINT @definition;
------------------------*/

CREATE PROCEDURE y
AS
BEGIN
  SELECT 1;
END;

DECLARE @RG_SC_VERSION BIGINT
SET @RG_SC_VERSION = 456
IF EXISTS (SELECT 1 FROM fn_listextendedproperty(N'SQLSourceControl Database Revision', NULL, NULL, NULL, NULL, NULL, NULL))
  EXEC sp_dropextendedproperty N'SQLSourceControl Database Revision', NULL, NULL, NULL, NULL, NULL, NULL
EXEC sp_addextendedproperty N'SQLSourceControl Database Revision', @RG_SC_VERSION, NULL, NULL, NULL, NULL, NULL, NULL

    If the last object is a table, then there is no problem with the sync script.

    If the last object is a procedure, then the object definition is mangled, and the version property is not updated.

    Is that enough information for you to reproduce the issue and avoid it in Migrations V2?

    Looking forward to the new version!
    Iain Elder, Skyscanner
    · Share on Twitter
  • James BJames B Posts: 1,124 Silver 4
    Ah, makes more sense, I think in my quick test the subsequent object was indeed a table.
    I did definitely observe the missing separator though. I'll let the team have a look over this thread. I think with the way V2 works (albeit my limited knowledge at this point!) it shouldn't be an issue either way, but they can verify that.
    Systems Software Engineer

    Redgate Software

    · Share on Twitter
  • freecell1freecell1 Posts: 33
    I had the same thing just happen to me except it was when the last (and only) object was a function. I was able to work around it by adding a bogus statement after the alter function part:

    -- this was the END from the alter function section
END

-- this was getting omitted.
GO

-- non-update to a random table 
UPDATE dbo.SearchParameter
SET DisplayName = DisplayName
WHERE 1 = 0;

-- this GO still got omitted but for some reason things worked.
GO
    · Share on Twitter
  • Sergio RSergio R Posts: 610 Rose Gold 5
    Hi,

    Since Migrations v2 has been deprecated in SQL Source Control 5 the bug report that was created as a result of this post is now closed.

    Thank you,
    Sergio
    Product Support Engineer
    Redgate Software Ltd
    Please see our Help Center for detailed guides on how to use our tools
    · Share on Twitter
Sign In or Register to comment.