Skip to content
This repository has been archived by the owner on Feb 9, 2024. It is now read-only.

DAC incorrectly sets containment type = Partial when restoring to SQL 2019 instance #36

Open
IanKemp opened this issue Feb 13, 2020 · 6 comments

Comments

@IanKemp
Copy link

IanKemp commented Feb 13, 2020

I'm generating a bacpac file from a SQL Azure hosted database (compatibility level SQL Server 2017) and restoring it to a SQL Server localdb instance. This was working fine, but then I upgraded my localdb instance to SQL 2019, and now the restore fails:

Creating deployment plan
Initializing deployment
Verifying deployment plan
Analyzing deployment plan
Importing package schema and data into database
Updating database
*** Error importing database:Could not import package.
Error SQL72014: .Net SqlClient Data Provider: Msg 12824, Level 16, State 1, Line 5 The sp_configure value 'contained database authentication' must be set to 1 in order to alter a contained database.  You may need to use RECONFIGURE to set the value_in_use.
Error SQL72045: Script execution error.  The executed script:
IF EXISTS (SELECT 1
           FROM   [master].[dbo].[sysdatabases]
           WHERE  [name] = N'$(DatabaseName)')
    BEGIN
        ALTER DATABASE [$(DatabaseName)]
            SET FILESTREAM(NON_TRANSACTED_ACCESS = OFF),
                CONTAINMENT = PARTIAL
            WITH ROLLBACK IMMEDIATE;
    END


Error SQL72014: .Net SqlClient Data Provider: Msg 5069, Level 16, State 1, Line 5 ALTER DATABASE statement failed.      Error SQL72045: Script execution error.  The executed script:
IF EXISTS (SELECT 1
           FROM   [master].[dbo].[sysdatabases]
           WHERE  [name] = N'$(DatabaseName)')
    BEGIN
        ALTER DATABASE [$(DatabaseName)]
            SET FILESTREAM(NON_TRANSACTED_ACCESS = OFF),
                CONTAINMENT = PARTIAL
            WITH ROLLBACK IMMEDIATE;
    END

The important point to note here is that the SQL Azure database has containment type = None so I have no idea why (what I presume to be) the generated script is trying to alter the destination DB to have CONTAINMENT = PARTIAL. I believe that SQLPackage is assuming that a 2019 DB must always be set to have containment type = Partial, rather than using the source database's containment type - i.e. this is a bug.

The arguments I'm passing to SQLPackage are simple: /a:Import /sf:"${bacpacFile}" /tcs:"${targetConnectionString}". This happens with both the latest .NET Framework (15.0.4627.2) and .NET Core (15.0.4630.1) versions of SQLPackage.

@IanKemp
Copy link
Author

IanKemp commented Feb 13, 2020

I just tried to import a BACPAC via SSMS's "Import Data-tier Application" wizard and received the same error message. So this does not look like a SQLPackage issue, but rather a DAC one.

image

@IanKemp IanKemp changed the title SQLPackage incorrectly sets containment type = Partial when it should be None during restore DAC incorrectly sets containment type = Partial when it should be None during bacpac restore Feb 13, 2020
@IanKemp IanKemp changed the title DAC incorrectly sets containment type = Partial when it should be None during bacpac restore DAC incorrectly sets containment type = Partial when it should be None during Azure SQL bacpac restore Feb 13, 2020
@IanKemp
Copy link
Author

IanKemp commented Feb 13, 2020

I believe the bug is in Microsoft.Data.Tools.Schema.Sql.dll, to be specific the Microsoft.Data.Tools.Schema.Sql.Deployment.Sql110DeploymentPlanGenerator#BuildAlterDatabaseStatements() method, in particular this block:

if (targetOptions != null && sourceOptions.Model.Platform == SqlPlatforms.SqlAzureV12 && SqlDeploymentPlanGenerator.IsPropertySupported(base.TargetModel.Platform, SqlDatabaseOptions.ContainmentClass) && !Sql110ModelComparer.CompareContainmentProperty(sourceOptions, targetOptions, SqlDatabaseOptions.ContainmentClass, null) && base.SupportsContainment)
{
    bool updateContainment = true;
    foreach (DatabaseOption option8 in setStatement.Options)
    {
        ContainmentDatabaseOption option2 = option8 as ContainmentDatabaseOption;
        if (option2 != null)
        {
            updateContainment = false;
            option2.Value = ContainmentOptionKind.Partial;
            break;
        }
    }
    if (updateContainment)
    {
        ContainmentDatabaseOption option = new ContainmentDatabaseOption
        {
            Value = ContainmentOptionKind.Partial
        };
        setStatement.Options.Add(option);
    }
}

I'm obviously no expert in this code, but it seems to me that either this block is being entered incorrectly (the outermost if has faulty logic and is evaluating when it should not be and/or the inner ifs in the foreach are not), or something upstream is setting a property incorrectly thus causing the if to be entered when it shouldn't be.

If this was an open-source project I would be able to pull the source, replicate the bug, make a fix, and submit a pull request... but it's not, and it's only due to the magic of ILSpy that I'm able to get this far.

@IanKemp
Copy link
Author

IanKemp commented Feb 13, 2020

This issue is present since at least 15.0.4538.1 (the oldest .NET Core Windows build of SQLPackage that is available for download).

@IanKemp IanKemp changed the title DAC incorrectly sets containment type = Partial when it should be None during Azure SQL bacpac restore DAC incorrectly sets containment type = Partial when restoring to SQL 2019 instance Feb 14, 2020
@IanKemp
Copy link
Author

IanKemp commented Feb 14, 2020

Issue present as far back as 15.0.4200.2 (from the oldest downloadable version of the 15.x DAC package I could find).

Issue also present in 14.0.3881.1 (newest version of the 14 series that does not give me an "unable to connect to to master or target server" error).

@IanKemp
Copy link
Author

IanKemp commented Mar 9, 2020

Significant time with ILSpy has confirmed that the issue is due to the aforementioned block of code. However I now believe that the block itself is correct, and the condition causing it to be entered is the problem.

In particular, it seems that the method Microsoft.Data.Tools.Schema.Sql.SchemaModel.Sql110ModelComparer#CompareContainmentProperty is buggy. It's comparing containment properties for two models, neither of which has containment set or required, yet is returning false (indicating that it thinks the containment between the two is/should be different) when it should be returning true.

I think this is because that method only considers the source containment for SqlAzureV12 when it should also be considering the target containment.

@llali
Copy link
Member

llali commented Jun 23, 2020

@IanKemp, One of the reasons DacFx changes target db to be containment is if the source db has contained user. Exporting a db with contained user cannot happen if target db is not changed to partial.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants