From 30d90abe1f2c39fe722be1830d2cf6702f9f2a7c Mon Sep 17 00:00:00 2001 From: Rob Ramsay Date: Wed, 10 Jun 2026 15:50:04 -0700 Subject: [PATCH 1/9] Schema Compare: surface Source/TargetPlatform on the result contract For Feature 1847587 (Schema Compare for Fabric Warehouse). The vscode-mssql client needs to know which DacFx platform a comparison ran under so it can display the dialect badge in the diff view header (e.g. `SqlDwUnified` for Fabric Warehouse vs. `Sql160` for Azure SQL Database). Today the result has no way to convey this without the client re-walking the DacFx model. Adds `SourcePlatform` and `TargetPlatform` (nullable `string`) to both `SchemaCompareResult` types (`Microsoft.SqlTools.SqlCore` for SSMS direct-call consumers; `Microsoft.SqlTools.ServiceLayer` for the JSON-RPC `schemaCompare/compare` response). Populated from `ComparisonResult.SourceModel.Version.ToString()` and the target equivalent after the comparison runs -- pass-through reads, no new I/O. Null-safe when the comparison didn't produce a model. Wired through `SchemaCompareOperation`: - New `SourcePlatform` / `TargetPlatform` properties on the operation set by `Execute()` from the DacFx `ComparisonResult` models. - `SchemaCompareService` reads these into the JSON-RPC `SchemaCompareResult` it returns to clients. Tests: two new unit tests in `SchemaCompareTests.cs` covering the contract shape (both `SchemaCompareResult` and the operation expose the properties, default to `null`, and accept writes). End-to-end behaviour against live comparisons is covered by integration tests for Feature 1847587. No DacFx API change. No version bump required for this commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Contracts/SchemaCompareRequest.cs | 13 +++++ .../SchemaCompare/SchemaCompareService.cs | 4 +- .../Contracts/SchemaCompareResults.cs | 13 +++++ .../SchemaCompare/SchemaCompareOperation.cs | 23 +++++++++ .../SchemaCompare/SchemaCompareTests.cs | 50 +++++++++++++++++++ 5 files changed, 102 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/Contracts/SchemaCompareRequest.cs b/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/Contracts/SchemaCompareRequest.cs index 38528dc096..6b903d67d7 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/Contracts/SchemaCompareRequest.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/Contracts/SchemaCompareRequest.cs @@ -49,6 +49,19 @@ public class SchemaCompareResult : ResultStatus public bool AreEqual { get; set; } public List Differences { get; set; } + + /// + /// The DacFx SqlServerVersion short name detected for the source endpoint + /// after the comparison runs (e.g. "Sql160", "SqlDwUnified", "SqlAzure"). Null if + /// the comparison did not produce a source model. + /// + public string SourcePlatform { get; set; } + + /// + /// The DacFx SqlServerVersion short name detected for the target endpoint. + /// Null if the comparison did not produce a target model. + /// + public string TargetPlatform { get; set; } } /// diff --git a/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareService.cs b/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareService.cs index 982826fe92..fbf6d0e1cb 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/SchemaCompareService.cs @@ -87,7 +87,9 @@ await requestContext.SendResult(new SchemaCompareResult() Success = operation.ComparisonResult.IsValid, ErrorMessage = operation.ErrorMessage, AreEqual = operation.ComparisonResult.IsEqual, - Differences = operation.Differences + Differences = operation.Differences, + SourcePlatform = operation.SourcePlatform, + TargetPlatform = operation.TargetPlatform }); // clean up cancellation action now that the operation is complete (using try remove to avoid exception) diff --git a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/Contracts/SchemaCompareResults.cs b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/Contracts/SchemaCompareResults.cs index 428c680443..24800e60c3 100644 --- a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/Contracts/SchemaCompareResults.cs +++ b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/Contracts/SchemaCompareResults.cs @@ -27,6 +27,19 @@ public class SchemaCompareResult : SchemaCompareResultBase /// List of differences found between source and target. /// public List Differences { get; set; } + + /// + /// The DacFx enum value + /// detected for the source endpoint (e.g. "Sql160", "SqlDwUnified", "SqlAzure"). + /// Null if the comparison did not produce a source model. + /// + public string SourcePlatform { get; set; } + + /// + /// The DacFx enum value + /// detected for the target endpoint. Null if the comparison did not produce a target model. + /// + public string TargetPlatform { get; set; } } /// diff --git a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs index deaeb38655..dc89820dcc 100644 --- a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs +++ b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs @@ -36,6 +36,21 @@ public class SchemaCompareOperation : IDisposable public List Differences; + /// + /// The DacFx enum value + /// detected for the source endpoint after the comparison runs (e.g. "Sql160", + /// "SqlDwUnified", "SqlAzure"). Null if the comparison did not produce a source model + /// (e.g. when ComparisonResult.SourceModel is null or has no Version). + /// + public string SourcePlatform { get; set; } + + /// + /// The DacFx enum value + /// detected for the target endpoint after the comparison runs. Null if the comparison + /// did not produce a target model. + /// + public string TargetPlatform { get; set; } + public SchemaCompareOperation(SchemaCompareParams parameters, ISchemaCompareConnectionProvider connectionProvider) { Validate.IsNotNull("parameters", parameters); @@ -117,6 +132,14 @@ public void Execute() } } + // Expose the platform the comparison actually ran under so clients can show + // the user which T-SQL dialect Schema Compare is using (e.g. "SqlDwUnified" + // when comparing Fabric Warehouse endpoints). Reading model.Version returns + // a SqlServerVersion enum; ToString() yields the short name that matches the + // DacFx Sql*SchemaCompareSettingsService chain (Sql160, SqlAzure, SqlDwUnified, ...). + this.SourcePlatform = this.ComparisonResult.SourceModel?.Version.ToString(); + this.TargetPlatform = this.ComparisonResult.TargetModel?.Version.ToString(); + // Appending the set of errors that are stopping the schema compare to the ErrorMessage // GetErrors return all type of warnings, and error messages. Only filtering the error type messages here var errorsList = ComparisonResult.GetErrors().Where(x => x.MessageType.Equals(DacMessageType.Error)).Select(e => e.Message).Distinct().ToList(); diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs index 45cc6ea238..d43650a9fb 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs @@ -130,5 +130,55 @@ private void ValidateTableCreation(string[] nameParts, string validationString) Assert.AreEqual(validObject1.NameParts[i], validResult1.Identifier.Parts[i]); } } + + [Test] + public void SchemaCompareResultExposesSourceAndTargetPlatformProperties() + { + // The SourcePlatform / TargetPlatform projection lets clients show the user + // which T-SQL dialect Schema Compare is actually running under (e.g. "SqlDwUnified" + // for Fabric Warehouse vs. "Sql160" for Azure SQL Database). The values are + // pass-through reads of ComparisonResult.SourceModel.Version.ToString() / + // TargetModel.Version.ToString(), so the meaningful unit-level guarantee is that + // the contract surface accepts and round-trips the values. End-to-end behavior + // for live comparisons is covered by integration tests. + SchemaCompareResult result = new SchemaCompareResult + { + OperationId = "op-1", + Success = true, + AreEqual = false, + SourcePlatform = "SqlDwUnified", + TargetPlatform = "SqlDwUnified", + }; + + Assert.AreEqual("SqlDwUnified", result.SourcePlatform); + Assert.AreEqual("SqlDwUnified", result.TargetPlatform); + + // Default values must be null so existing clients that ignore the field continue + // to behave as before and JSON serializers omit the property when the comparison + // did not produce a model (e.g. failed validation). + SchemaCompareResult defaultResult = new SchemaCompareResult(); + Assert.IsNull(defaultResult.SourcePlatform); + Assert.IsNull(defaultResult.TargetPlatform); + } + + [Test] + public void SchemaCompareOperationExposesSourceAndTargetPlatformProperties() + { + // The operation hoists Source/TargetPlatform from ComparisonResult.Source/TargetModel + // after Execute() so SchemaCompareService can wire it into the SchemaCompareResult + // contract without re-walking the DacFx model on the JSON-RPC layer. + SchemaCompareParams parameters = new SchemaCompareParams { OperationId = "op-2" }; + SchemaCompareOperation operation = new SchemaCompareOperation(parameters, connectionProvider: null); + + // Properties exist and are writable by the Execute() path. + Assert.IsNull(operation.SourcePlatform); + Assert.IsNull(operation.TargetPlatform); + + operation.SourcePlatform = "Sql160"; + operation.TargetPlatform = "SqlDwUnified"; + + Assert.AreEqual("Sql160", operation.SourcePlatform); + Assert.AreEqual("SqlDwUnified", operation.TargetPlatform); + } } } From 23a8ef56315083e7a62bd560f24a9d498d3f8835 Mon Sep 17 00:00:00 2001 From: Rob Ramsay Date: Fri, 12 Jun 2026 12:17:24 -0700 Subject: [PATCH 2/9] Schema Compare: read platform from DSP instead of SqlServerVersion The public TSqlModel.Version surface maps SqlPlatforms.SqlDwUnified to SqlServerVersion.Sql150 inside DacFx (InternalModelUtils.CalculateVersionsForPlatform), so reading .Version.ToString() always returned 'Sql150' for Fabric Warehouse models and mislabelled the platform pill in Schema Compare. Read the comparison's normalized DSP from SchemaComparisonResult.DataModel.DatabaseSchemaProvider.Platform via reflection. DatabaseSchemaProvider is internal-abstract in DacFx but its .Platform property returns the public SqlPlatforms enum, whose values map directly to the DSP-name strings clients expect ('Sql160', 'SqlAzure', 'SqlDwUnified', etc.). Lookups are cached statically; any reflection failure is logged and falls back to a null platform string (the pill silently hides). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SchemaCompare/SchemaCompareOperation.cs | 150 ++++++++++++++++-- 1 file changed, 138 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs index dc89820dcc..4e950aa488 100644 --- a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs +++ b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs @@ -11,6 +11,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Threading; namespace Microsoft.SqlTools.SqlCore.SchemaCompare @@ -37,17 +38,17 @@ public class SchemaCompareOperation : IDisposable public List Differences; /// - /// The DacFx enum value - /// detected for the source endpoint after the comparison runs (e.g. "Sql160", - /// "SqlDwUnified", "SqlAzure"). Null if the comparison did not produce a source model - /// (e.g. when ComparisonResult.SourceModel is null or has no Version). + /// The platform the comparison ran under (DacFx SqlPlatforms enum name, e.g. + /// "Sql160", "SqlAzureV12", "SqlDwUnified"). Sourced from the comparison's + /// DatabaseSchemaProvider.Platform (the unified DSP the comparison normalized + /// to). Source and Target carry the same value because DacFx runs a single comparison + /// under a single DSP — see . Null if the + /// platform could not be detected (e.g. the comparison was never run). /// public string SourcePlatform { get; set; } /// - /// The DacFx enum value - /// detected for the target endpoint after the comparison runs. Null if the comparison - /// did not produce a target model. + /// The platform the comparison ran under. See . /// public string TargetPlatform { get; set; } @@ -134,11 +135,23 @@ public void Execute() // Expose the platform the comparison actually ran under so clients can show // the user which T-SQL dialect Schema Compare is using (e.g. "SqlDwUnified" - // when comparing Fabric Warehouse endpoints). Reading model.Version returns - // a SqlServerVersion enum; ToString() yields the short name that matches the - // DacFx Sql*SchemaCompareSettingsService chain (Sql160, SqlAzure, SqlDwUnified, ...). - this.SourcePlatform = this.ComparisonResult.SourceModel?.Version.ToString(); - this.TargetPlatform = this.ComparisonResult.TargetModel?.Version.ToString(); + // when comparing Fabric Warehouse endpoints). + // + // We CANNOT use ComparisonResult.SourceModel.Version because DacFx maps the + // SqlDwUnified platform to SqlServerVersion.Sql150 in + // InternalModelUtils.CalculateVersionsForPlatform (it predates the + // SqlServerVersion.SqlDwUnified enum value). The TSqlModel.Version surface + // therefore reports "Sql150" for every Fabric Warehouse model, which would + // mislabel the platform pill in the UI. + // + // Instead, read the comparison's normalized DSP from + // SchemaComparisonResult.DataModel.DatabaseSchemaProvider.Platform via + // reflection. DatabaseSchemaProvider is internal-abstract in DacFx, but + // .Platform is a public abstract SqlPlatforms property (a public enum), and + // the cascade fix in PR 2143938 confirms this value is reliably populated. + string comparisonPlatform = TryGetComparisonPlatform(this.ComparisonResult); + this.SourcePlatform = comparisonPlatform; + this.TargetPlatform = comparisonPlatform; // Appending the set of errors that are stopping the schema compare to the ErrorMessage // GetErrors return all type of warnings, and error messages. Only filtering the error type messages here @@ -157,5 +170,118 @@ public void Execute() } internal event EventHandler schemaCompareStarted; + + // Cached reflection accessors for SchemaComparisonResult.DataModel.DatabaseSchemaProvider.Platform. + // Resolved lazily and atomically on first use so we pay the lookup cost once per process. + private static PropertyInfo s_dataModelProp; + private static PropertyInfo s_dspProp; + private static PropertyInfo s_platformProp; + private static bool s_reflectionInitFailed; + private static readonly object s_reflectionInitLock = new object(); + + /// + /// Returns the comparison's DSP platform as a string (e.g. "Sql160", "SqlDwUnified") + /// by reflecting into the internal SchemaCompareDataModel.DatabaseSchemaProvider + /// reachable from . Returns null if any step + /// of the lookup fails or returns null — callers should treat null as "unknown" and not + /// surface an error to the user, since the platform pill is a diagnostic affordance, + /// not a blocking signal. + /// + /// + /// This is a reflection workaround for the lack of a public DacFx accessor exposing + /// the comparison's platform. TSqlModel.Version would be the natural API but + /// returns Sql150 for Fabric Warehouse models (see + /// InternalModelUtils.CalculateVersionsForPlatform). If DacFx adds a public + /// SchemaComparisonResult.Platform property in the future, replace this with + /// the direct call. + /// + private static string TryGetComparisonPlatform(SchemaComparisonResult result) + { + if (result == null) + { + return null; + } + + try + { + if (!EnsureReflectionMembers(result.GetType())) + { + return null; + } + + object dataModel = s_dataModelProp.GetValue(result); + if (dataModel == null) + { + return null; + } + + PropertyInfo dspProp = s_dspProp ?? dataModel.GetType().GetProperty("DatabaseSchemaProvider"); + if (dspProp == null) + { + return null; + } + s_dspProp = dspProp; + + object dsp = dspProp.GetValue(dataModel); + if (dsp == null) + { + return null; + } + + PropertyInfo platformProp = s_platformProp ?? dsp.GetType().GetProperty("Platform"); + if (platformProp == null) + { + return null; + } + s_platformProp = platformProp; + + object platformValue = platformProp.GetValue(dsp); + return platformValue?.ToString(); + } + catch (Exception ex) + { + // Reflection failures are non-fatal: the platform pill simply won't render + // and the user keeps their compare results. Log so the failure is diagnosable + // from the STS log file without surfacing to the UI. + Logger.Warning(string.Format("Schema compare: failed to detect comparison platform via reflection: {0}", ex.Message)); + return null; + } + } + + private static bool EnsureReflectionMembers(Type resultType) + { + if (s_dataModelProp != null) + { + return true; + } + if (s_reflectionInitFailed) + { + return false; + } + + lock (s_reflectionInitLock) + { + if (s_dataModelProp != null) + { + return true; + } + if (s_reflectionInitFailed) + { + return false; + } + + PropertyInfo prop = resultType.GetProperty( + "DataModel", + BindingFlags.NonPublic | BindingFlags.Instance); + if (prop == null) + { + s_reflectionInitFailed = true; + Logger.Warning("Schema compare: SchemaComparisonResult.DataModel property not found via reflection; platform pill will be unavailable."); + return false; + } + s_dataModelProp = prop; + return true; + } + } } } From aa9a076176ac852ca51b88f44c70344d39824835 Mon Sep 17 00:00:00 2001 From: Rob Ramsay Date: Fri, 12 Jun 2026 12:38:07 -0700 Subject: [PATCH 3/9] Schema Compare: keep Fabric Warehouse constraint scripts under table diffs CreateDiffEntry stripped any child script that started with `alter` to avoid duplicating constraint DDL that the parent's CREATE TABLE already inlined for legacy SQL Server. SqlDwUnified (Fabric Warehouse) never inlines PK/FK/UNIQUE/ CHECK/DEFAULT constraints into CREATE TABLE; they are always emitted as standalone `ALTER TABLE ... ADD CONSTRAINT ...` statements. The filter therefore discarded the only script that defined the constraint, so the diff editor showed CREATE TABLE without the PK and the generated script was missing the PK definition entirely. Detect constraint children (PK/FK/UNIQUE/CHECK/DEFAULT) under a SqlDwUnified comparison and preserve their scripts in that case. Behavior is unchanged for all other platforms. Move the cached reflection-based platform detector from SchemaCompareOperation into SchemaCompareUtils so CreateDiffEntry can use it; add a ConditionalWeakTable cache keyed by SchemaComparisonResult so the recursive walk pays the reflection cost once per comparison, not once per diff entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SchemaCompare/SchemaCompareOperation.cs | 119 +---------- .../SchemaCompare/SchemaCompareUtils.cs | 190 +++++++++++++++++- 2 files changed, 191 insertions(+), 118 deletions(-) diff --git a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs index 4e950aa488..14f349080f 100644 --- a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs +++ b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareOperation.cs @@ -11,7 +11,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Reflection; using System.Threading; namespace Microsoft.SqlTools.SqlCore.SchemaCompare @@ -42,7 +41,8 @@ public class SchemaCompareOperation : IDisposable /// "Sql160", "SqlAzureV12", "SqlDwUnified"). Sourced from the comparison's /// DatabaseSchemaProvider.Platform (the unified DSP the comparison normalized /// to). Source and Target carry the same value because DacFx runs a single comparison - /// under a single DSP — see . Null if the + /// under a single DSP — see . + /// Null if the /// platform could not be detected (e.g. the comparison was never run). /// public string SourcePlatform { get; set; } @@ -149,7 +149,7 @@ public void Execute() // reflection. DatabaseSchemaProvider is internal-abstract in DacFx, but // .Platform is a public abstract SqlPlatforms property (a public enum), and // the cascade fix in PR 2143938 confirms this value is reliably populated. - string comparisonPlatform = TryGetComparisonPlatform(this.ComparisonResult); + string comparisonPlatform = SchemaCompareUtils.GetComparisonPlatform(this.ComparisonResult); this.SourcePlatform = comparisonPlatform; this.TargetPlatform = comparisonPlatform; @@ -170,118 +170,5 @@ public void Execute() } internal event EventHandler schemaCompareStarted; - - // Cached reflection accessors for SchemaComparisonResult.DataModel.DatabaseSchemaProvider.Platform. - // Resolved lazily and atomically on first use so we pay the lookup cost once per process. - private static PropertyInfo s_dataModelProp; - private static PropertyInfo s_dspProp; - private static PropertyInfo s_platformProp; - private static bool s_reflectionInitFailed; - private static readonly object s_reflectionInitLock = new object(); - - /// - /// Returns the comparison's DSP platform as a string (e.g. "Sql160", "SqlDwUnified") - /// by reflecting into the internal SchemaCompareDataModel.DatabaseSchemaProvider - /// reachable from . Returns null if any step - /// of the lookup fails or returns null — callers should treat null as "unknown" and not - /// surface an error to the user, since the platform pill is a diagnostic affordance, - /// not a blocking signal. - /// - /// - /// This is a reflection workaround for the lack of a public DacFx accessor exposing - /// the comparison's platform. TSqlModel.Version would be the natural API but - /// returns Sql150 for Fabric Warehouse models (see - /// InternalModelUtils.CalculateVersionsForPlatform). If DacFx adds a public - /// SchemaComparisonResult.Platform property in the future, replace this with - /// the direct call. - /// - private static string TryGetComparisonPlatform(SchemaComparisonResult result) - { - if (result == null) - { - return null; - } - - try - { - if (!EnsureReflectionMembers(result.GetType())) - { - return null; - } - - object dataModel = s_dataModelProp.GetValue(result); - if (dataModel == null) - { - return null; - } - - PropertyInfo dspProp = s_dspProp ?? dataModel.GetType().GetProperty("DatabaseSchemaProvider"); - if (dspProp == null) - { - return null; - } - s_dspProp = dspProp; - - object dsp = dspProp.GetValue(dataModel); - if (dsp == null) - { - return null; - } - - PropertyInfo platformProp = s_platformProp ?? dsp.GetType().GetProperty("Platform"); - if (platformProp == null) - { - return null; - } - s_platformProp = platformProp; - - object platformValue = platformProp.GetValue(dsp); - return platformValue?.ToString(); - } - catch (Exception ex) - { - // Reflection failures are non-fatal: the platform pill simply won't render - // and the user keeps their compare results. Log so the failure is diagnosable - // from the STS log file without surfacing to the UI. - Logger.Warning(string.Format("Schema compare: failed to detect comparison platform via reflection: {0}", ex.Message)); - return null; - } - } - - private static bool EnsureReflectionMembers(Type resultType) - { - if (s_dataModelProp != null) - { - return true; - } - if (s_reflectionInitFailed) - { - return false; - } - - lock (s_reflectionInitLock) - { - if (s_dataModelProp != null) - { - return true; - } - if (s_reflectionInitFailed) - { - return false; - } - - PropertyInfo prop = resultType.GetProperty( - "DataModel", - BindingFlags.NonPublic | BindingFlags.Instance); - if (prop == null) - { - s_reflectionInitFailed = true; - Logger.Warning("Schema compare: SchemaComparisonResult.DataModel property not found via reflection; platform pill will be unavailable."); - return false; - } - s_dataModelProp = prop; - return true; - } - } } } diff --git a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs index bf2bf804ec..439a3ecabf 100644 --- a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs +++ b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs @@ -6,11 +6,14 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reflection; +using System.Runtime.CompilerServices; using System.Text.RegularExpressions; using Microsoft.SqlServer.Dac; using Microsoft.SqlServer.Dac.Compare; using Microsoft.SqlServer.Dac.Model; using Microsoft.SqlTools.SqlCore.SchemaCompare.Contracts; +using Microsoft.SqlTools.Utility; namespace Microsoft.SqlTools.SqlCore.SchemaCompare { @@ -22,6 +25,23 @@ internal static class SchemaCompareUtils { private static readonly Regex ExcessWhitespaceRegex = new Regex(" {2,}", RegexOptions.Compiled); + // The DacFx full type-name suffixes for the constraint kinds that, on SqlDwUnified + // (Fabric Warehouse), are always emitted as standalone "ALTER TABLE ... ADD CONSTRAINT" + // statements rather than inlined into CREATE TABLE. We need to preserve their child + // scripts so the diff editor and aggregated generated script include the constraint + // definitions. Match SchemaComparisonExcludedObjectId.TypeName which is the + // full .NET type name like "Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlPrimaryKeyConstraint". + private static readonly string[] ConstraintTypeNameSuffixes = new[] + { + "PrimaryKeyConstraint", + "ForeignKeyConstraint", + "UniqueConstraint", + "CheckConstraint", + "DefaultConstraint", + }; + + private const string SqlDwUnifiedPlatformName = "SqlDwUnified"; + internal static DiffEntry CreateDiffEntry(SchemaDifference difference, DiffEntry parent, SchemaComparisonResult schemaComparisonResult) { if (difference == null) @@ -51,6 +71,15 @@ internal static DiffEntry CreateDiffEntry(SchemaDifference difference, DiffEntry if (difference.DifferenceType == SchemaDifferenceType.Object) { + // Fabric Warehouse (SqlDwUnified) never inlines PK/FK/UNIQUE/CHECK/DEFAULT + // constraints into CREATE TABLE — every constraint is emitted as a standalone + // "ALTER TABLE ... ADD CONSTRAINT ..." script. The legacy filter below skips + // child scripts that start with "alter" on the assumption that they're + // duplicates of inline constraints already present in the parent's CREATE. + // For SqlDwUnified that assumption is wrong: the ALTER script is the ONLY + // place the constraint is defined, so we must keep it. + bool keepAlterScript = IsConstraintChildOnSqlDwUnified(diffEntry, schemaComparisonResult); + // set source and target scripts if (difference.SourceObject != null) { @@ -58,7 +87,7 @@ internal static DiffEntry CreateDiffEntry(SchemaDifference difference, DiffEntry // Child scripts that do not use alter need to be added if they are being changed, ex: "EXECUTE sp_addextendedproperty...". // Don't add scripts that start with alter because those are handled by a top level element's create - if (!sourceScript.ToLowerInvariant().StartsWith("alter")) + if (keepAlterScript || !sourceScript.ToLowerInvariant().StartsWith("alter")) { diffEntry.SourceScript = FormatScript(sourceScript); } @@ -69,7 +98,7 @@ internal static DiffEntry CreateDiffEntry(SchemaDifference difference, DiffEntry // Child scripts that do not use alter need to be added if they are being changed, ex: "EXECUTE sp_addextendedproperty...". // Don't add scripts that start with alter because those are handled by a top level element's create - if (!targetScript.ToLowerInvariant().StartsWith("alter")) + if (keepAlterScript || !targetScript.ToLowerInvariant().StartsWith("alter")) { diffEntry.TargetScript = FormatScript(targetScript); } @@ -86,6 +115,33 @@ internal static DiffEntry CreateDiffEntry(SchemaDifference difference, DiffEntry return diffEntry; } + private static bool IsConstraintChildOnSqlDwUnified(DiffEntry diffEntry, SchemaComparisonResult schemaComparisonResult) + { + string typeName = diffEntry.SourceObjectType ?? diffEntry.TargetObjectType; + if (string.IsNullOrEmpty(typeName)) + { + return false; + } + + bool isConstraint = false; + foreach (string suffix in ConstraintTypeNameSuffixes) + { + if (typeName.EndsWith(suffix, StringComparison.Ordinal)) + { + isConstraint = true; + break; + } + } + if (!isConstraint) + { + return false; + } + + string platform = GetComparisonPlatform(schemaComparisonResult); + return string.Equals(platform, SqlDwUnifiedPlatformName, StringComparison.Ordinal); + } + + internal static SchemaComparisonExcludedObjectId CreateExcludedObject(SchemaCompareObjectId sourceObj) { try @@ -155,5 +211,135 @@ internal static string FormatScript(string script) } return script; } + + // Cached reflection accessors for SchemaComparisonResult.DataModel.DatabaseSchemaProvider.Platform. + // Resolved lazily and atomically on first use so we pay the lookup cost once per process. + private static PropertyInfo s_dataModelProp; + private static PropertyInfo s_dspProp; + private static PropertyInfo s_platformProp; + private static bool s_reflectionInitFailed; + private static readonly object s_reflectionInitLock = new object(); + + // Per-result cache so CreateDiffEntry's recursive walk does not re-reflect for every child. + // ConditionalWeakTable doesn't allow null values, so we sentinel "unknown" as empty string. + private static readonly ConditionalWeakTable s_platformByResult = + new ConditionalWeakTable(); + + /// + /// Returns the comparison's DSP platform as a string (e.g. "Sql160", "SqlDwUnified") + /// by reflecting into the internal SchemaCompareDataModel.DatabaseSchemaProvider + /// reachable from . Returns null if any step + /// of the lookup fails or returns null. Result is cached per SchemaComparisonResult + /// instance so repeated calls are O(1) after the first. + /// + /// + /// This is a reflection workaround for the lack of a public DacFx accessor exposing + /// the comparison's platform. TSqlModel.Version would be the natural API but + /// returns Sql150 for Fabric Warehouse models (see + /// InternalModelUtils.CalculateVersionsForPlatform). If DacFx adds a public + /// SchemaComparisonResult.Platform property in the future, replace this with + /// the direct call. + /// + internal static string GetComparisonPlatform(SchemaComparisonResult result) + { + if (result == null) + { + return null; + } + + if (s_platformByResult.TryGetValue(result, out string cached)) + { + return string.IsNullOrEmpty(cached) ? null : cached; + } + + string platform = TryGetComparisonPlatformCore(result); + // ConditionalWeakTable rejects nulls, so store empty for "unknown". + s_platformByResult.Add(result, platform ?? string.Empty); + return platform; + } + + private static string TryGetComparisonPlatformCore(SchemaComparisonResult result) + { + try + { + if (!EnsureReflectionMembers(result.GetType())) + { + return null; + } + + object dataModel = s_dataModelProp.GetValue(result); + if (dataModel == null) + { + return null; + } + + PropertyInfo dspProp = s_dspProp ?? dataModel.GetType().GetProperty("DatabaseSchemaProvider"); + if (dspProp == null) + { + return null; + } + s_dspProp = dspProp; + + object dsp = dspProp.GetValue(dataModel); + if (dsp == null) + { + return null; + } + + PropertyInfo platformProp = s_platformProp ?? dsp.GetType().GetProperty("Platform"); + if (platformProp == null) + { + return null; + } + s_platformProp = platformProp; + + object platformValue = platformProp.GetValue(dsp); + return platformValue?.ToString(); + } + catch (Exception ex) + { + // Reflection failures are non-fatal: the platform pill simply won't render + // and the user keeps their compare results. Log so the failure is diagnosable + // from the STS log file without surfacing to the UI. + Logger.Warning(string.Format("Schema compare: failed to detect comparison platform via reflection: {0}", ex.Message)); + return null; + } + } + + private static bool EnsureReflectionMembers(Type resultType) + { + if (s_dataModelProp != null) + { + return true; + } + if (s_reflectionInitFailed) + { + return false; + } + + lock (s_reflectionInitLock) + { + if (s_dataModelProp != null) + { + return true; + } + if (s_reflectionInitFailed) + { + return false; + } + + PropertyInfo prop = resultType.GetProperty( + "DataModel", + BindingFlags.NonPublic | BindingFlags.Instance); + if (prop == null) + { + s_reflectionInitFailed = true; + Logger.Warning("Schema compare: SchemaComparisonResult.DataModel property not found via reflection; platform pill will be unavailable."); + return false; + } + s_dataModelProp = prop; + return true; + } + } } } From 40f96df2552a245ffd4c6bcb9aae7395ec71cc4d Mon Sep 17 00:00:00 2001 From: Rob Ramsay Date: Tue, 16 Jun 2026 11:50:08 -0700 Subject: [PATCH 4/9] Schema Compare: add Fabric Warehouse unit + integration tests (Feature 1847587) Implements the test design from the Schema Compare for Fabric Warehouse spec (Feature 1847587). Unit tests (SchemaCompareTests.cs): Extracts SchemaCompareUtils.ShouldPreserveAlterScriptForConstraint(typeName, platform) as a pure helper so the SqlDwUnified strip-on-alter carve-out is testable without manufacturing a sealed DacFx SchemaComparisonResult graph. Existing IsConstraintChildOnSqlDwUnified is now a thin wrapper. Adds 18 TestCase rows covering every constraint suffix under SqlDwUnified, non-constraint types under SqlDwUnified, every non-Fabric platform string a comparison can emit (Sql160/Sql150/SqlAzure/SqlAzureV12/SqlDw), null/empty type and platform inputs, substring-vs-suffix guard, and GetComparisonPlatform null-result short-circuit. Integration tests (SchemaCompareFabricTests.cs): Full 4-endpoint-pair x 7-scenario matrix (28 cells) via [TestCaseSource]. Scenarios: AllSupportedTypes, CompatibleAlterColumn, IncompatibleAlterColumn, PrimaryKey, Unique, ForeignKey, SelectionScope. Endpoint pairs: DacpacToDacpac, DacpacToProject, ProjectToDacpac, ProjectToProject. Fabric dacpacs are built offline at fixture setup via new TSqlModel(SqlServerVersion.SqlDwUnified, ...) + DacPackageExtensions.BuildPackage so the suite requires no live Fabric Warehouse. Per-cell assertions (per spec lines 309-316): comparison validity + IsEqual=false + Differences.Any(); SourcePlatform == TargetPlatform == SqlDwUnified; constraints folded under parent SqlTable diff (not surfaced as top-level entries); constraint child carries ADD CONSTRAINT NONCLUSTERED NOT ENFORCED script; selection-scope IncludeOnly(OrdersScope) yields a Generate Script output that references zero unrelated tables or constraints (locks in the Prototype 3 empirical regression that motivated DacFx PR 2143938); project-target round-trip converges to IsEqual after publish. Live Fabric publish (per-cell assertion 7) is intentionally manual-only per the spec; it is exercised by the WSR matrix, not automated here. Fixtures (test/.../SchemaCompare/Fabric/*.sql): 14 minimal T-SQL scripts confined to the Fabric-supported data type list, with all constraints scripted as NONCLUSTERED NOT ENFORCED. Plus emptyFabricTemplate.sqlproj with DSP=SqlDwUnifiedDatabaseSchemaProvider for the Project endpoint cases. Requires DacFx >= 170.5.20 (the published build that ships PR 2143938's cascade fix). The sts-version-bump that flips Packages.props onto that DacFx is a separate change and is not included here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SchemaCompare/SchemaCompareUtils.cs | 27 +- .../Fabric/FabricAllTypes_Source.sql | 24 + .../Fabric/FabricAllTypes_Target.sql | 21 + .../Fabric/FabricForeignKey_Source.sql | 17 + .../Fabric/FabricForeignKey_Target.sql | 12 + .../Fabric/FabricNarrowColumn_Source.sql | 9 + .../Fabric/FabricNarrowColumn_Target.sql | 6 + .../Fabric/FabricPrimaryKey_Source.sql | 9 + .../Fabric/FabricPrimaryKey_Target.sql | 5 + .../Fabric/FabricSelectionScope_Source.sql | 20 + .../Fabric/FabricSelectionScope_Target.sql | 10 + .../Fabric/FabricUnique_Source.sql | 9 + .../Fabric/FabricUnique_Target.sql | 5 + .../Fabric/FabricWidenColumn_Source.sql | 12 + .../Fabric/FabricWidenColumn_Target.sql | 8 + .../SchemaCompare/SchemaCompareFabricTests.cs | 591 ++++++++++++++++++ .../SqlProjects/emptyFabricTemplate.sqlproj | 13 + .../SchemaCompare/SchemaCompareTests.cs | 102 +++ 18 files changed, 897 insertions(+), 3 deletions(-) create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricAllTypes_Source.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricAllTypes_Target.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricForeignKey_Source.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricForeignKey_Target.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricNarrowColumn_Source.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricNarrowColumn_Target.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricPrimaryKey_Source.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricPrimaryKey_Target.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricSelectionScope_Source.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricSelectionScope_Target.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricUnique_Source.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricUnique_Target.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricWidenColumn_Source.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricWidenColumn_Target.sql create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs create mode 100644 test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SqlProjects/emptyFabricTemplate.sqlproj diff --git a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs index 439a3ecabf..20572b7256 100644 --- a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs +++ b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs @@ -118,7 +118,29 @@ internal static DiffEntry CreateDiffEntry(SchemaDifference difference, DiffEntry private static bool IsConstraintChildOnSqlDwUnified(DiffEntry diffEntry, SchemaComparisonResult schemaComparisonResult) { string typeName = diffEntry.SourceObjectType ?? diffEntry.TargetObjectType; - if (string.IsNullOrEmpty(typeName)) + string platform = GetComparisonPlatform(schemaComparisonResult); + return ShouldPreserveAlterScriptForConstraint(typeName, platform); + } + + /// + /// Pure helper: decides whether a diff-entry's "starts-with-alter" script must be + /// preserved instead of stripped. Returns true when the given DacFx object type name + /// is one of the constraint kinds we fold under a parent table + /// () and the comparison is running under the + /// Fabric Warehouse DSP (). All other inputs + /// — non-constraint types, non-Fabric platforms, null/empty type or platform — + /// return false, preserving the legacy strip-on-alter behaviour for SQL Server, + /// Azure SQL, and Synapse comparisons. + /// + /// + /// Extracted from so it can be unit + /// tested without manufacturing a real graph + /// (DacFx's comparison types are sealed with no public constructor that yields a + /// usable platform projection). + /// + internal static bool ShouldPreserveAlterScriptForConstraint(string objectTypeName, string platform) + { + if (string.IsNullOrEmpty(objectTypeName)) { return false; } @@ -126,7 +148,7 @@ private static bool IsConstraintChildOnSqlDwUnified(DiffEntry diffEntry, SchemaC bool isConstraint = false; foreach (string suffix in ConstraintTypeNameSuffixes) { - if (typeName.EndsWith(suffix, StringComparison.Ordinal)) + if (objectTypeName.EndsWith(suffix, StringComparison.Ordinal)) { isConstraint = true; break; @@ -137,7 +159,6 @@ private static bool IsConstraintChildOnSqlDwUnified(DiffEntry diffEntry, SchemaC return false; } - string platform = GetComparisonPlatform(schemaComparisonResult); return string.Equals(platform, SqlDwUnifiedPlatformName, StringComparison.Ordinal); } diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricAllTypes_Source.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricAllTypes_Source.sql new file mode 100644 index 0000000000..7a4b5cd52f --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricAllTypes_Source.sql @@ -0,0 +1,24 @@ +-- Fabric Warehouse (SqlDwUnified) source fixture covering the supported data types +-- exercised by SchemaCompareFabricTests. Keep types confined to the Fabric-supported +-- list from +-- https://learn.microsoft.com/en-us/fabric/data-warehouse/data-types +-- so the patched-DSP dacpac built from this script loads under SqlDwUnified without +-- DacFx complaining about unsupported types. +CREATE TABLE [dbo].[AllTypes] +( + [BitCol] BIT NOT NULL, + [SmallIntCol] SMALLINT NOT NULL, + [IntCol] INT NOT NULL, + [BigIntCol] BIGINT NOT NULL, + [DecimalCol] DECIMAL(10, 2) NOT NULL, + [NumericCol] NUMERIC(10, 2) NOT NULL, + [FloatCol] FLOAT(53) NOT NULL, + [RealCol] REAL NOT NULL, + [DateCol] DATE NOT NULL, + [TimeCol] TIME(6) NOT NULL, + [DateTime2Col] DATETIME2(6) NOT NULL, + [CharCol] CHAR(10) NOT NULL, + [VarCharCol] VARCHAR(100) NOT NULL, + [VarBinaryCol] VARBINARY(100) NULL, + [UniqueIdentifierCol] UNIQUEIDENTIFIER NOT NULL +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricAllTypes_Target.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricAllTypes_Target.sql new file mode 100644 index 0000000000..f173dec399 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricAllTypes_Target.sql @@ -0,0 +1,21 @@ +-- Pair with FabricAllTypes_Source.sql but with one column removed (UniqueIdentifierCol) +-- so the comparison produces a real difference. Asserts (a) all supported types load +-- under SqlDwUnified without DSP rejection, and (b) the comparison detects the +-- missing column as an Object/Change diff. +CREATE TABLE [dbo].[AllTypes] +( + [BitCol] BIT NOT NULL, + [SmallIntCol] SMALLINT NOT NULL, + [IntCol] INT NOT NULL, + [BigIntCol] BIGINT NOT NULL, + [DecimalCol] DECIMAL(10, 2) NOT NULL, + [NumericCol] NUMERIC(10, 2) NOT NULL, + [FloatCol] FLOAT(53) NOT NULL, + [RealCol] REAL NOT NULL, + [DateCol] DATE NOT NULL, + [TimeCol] TIME(6) NOT NULL, + [DateTime2Col] DATETIME2(6) NOT NULL, + [CharCol] CHAR(10) NOT NULL, + [VarCharCol] VARCHAR(100) NOT NULL, + [VarBinaryCol] VARBINARY(100) NULL +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricForeignKey_Source.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricForeignKey_Source.sql new file mode 100644 index 0000000000..6287f44234 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricForeignKey_Source.sql @@ -0,0 +1,17 @@ +-- Source for the FOREIGN KEY scenario. Two tables — Orders references Customers via +-- a NOT ENFORCED FK. Pair with FabricForeignKey_Target.sql which omits the FK so it +-- appears as an Add child of the Orders table diff. Cross-table dependency ordering +-- must be honoured in the generated script. +CREATE TABLE [dbo].[CustomersFk] +( + [CustomerId] INT NOT NULL, + CONSTRAINT [PK_CustomersFk] PRIMARY KEY NONCLUSTERED ([CustomerId]) NOT ENFORCED +); +GO +CREATE TABLE [dbo].[OrdersFk] +( + [OrderId] INT NOT NULL, + [CustomerId] INT NOT NULL, + CONSTRAINT [PK_OrdersFk] PRIMARY KEY NONCLUSTERED ([OrderId]) NOT ENFORCED, + CONSTRAINT [FK_OrdersFk_CustomersFk] FOREIGN KEY ([CustomerId]) REFERENCES [dbo].[CustomersFk] ([CustomerId]) NOT ENFORCED +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricForeignKey_Target.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricForeignKey_Target.sql new file mode 100644 index 0000000000..58d330db96 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricForeignKey_Target.sql @@ -0,0 +1,12 @@ +CREATE TABLE [dbo].[CustomersFk] +( + [CustomerId] INT NOT NULL, + CONSTRAINT [PK_CustomersFk] PRIMARY KEY NONCLUSTERED ([CustomerId]) NOT ENFORCED +); +GO +CREATE TABLE [dbo].[OrdersFk] +( + [OrderId] INT NOT NULL, + [CustomerId] INT NOT NULL, + CONSTRAINT [PK_OrdersFk] PRIMARY KEY NONCLUSTERED ([OrderId]) NOT ENFORCED +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricNarrowColumn_Source.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricNarrowColumn_Source.sql new file mode 100644 index 0000000000..73003d0bb3 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricNarrowColumn_Source.sql @@ -0,0 +1,9 @@ +-- Source for the narrowing / type-incompatible ALTER COLUMN scenario. Pair with +-- FabricNarrowColumn_Target.sql: BIGINT→INT and VARCHAR(200)→VARCHAR(10). DacFx is +-- expected to emit ALTER COLUMN diffs accompanied by data-loss warnings. +CREATE TABLE [dbo].[NarrowColumns] +( + [Id] INT NOT NULL, + [WideCount] BIGINT NOT NULL, + [LongName] VARCHAR(200) NOT NULL +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricNarrowColumn_Target.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricNarrowColumn_Target.sql new file mode 100644 index 0000000000..aa9c08a799 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricNarrowColumn_Target.sql @@ -0,0 +1,6 @@ +CREATE TABLE [dbo].[NarrowColumns] +( + [Id] INT NOT NULL, + [WideCount] INT NOT NULL, + [LongName] VARCHAR(10) NOT NULL +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricPrimaryKey_Source.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricPrimaryKey_Source.sql new file mode 100644 index 0000000000..3981ccf745 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricPrimaryKey_Source.sql @@ -0,0 +1,9 @@ +-- Source for the PRIMARY KEY scenario. Pair with FabricPrimaryKey_Target.sql which +-- has no PK so the PK appears as an Add child of the parent table's diff. PKs in +-- Fabric Warehouse must be NONCLUSTERED NOT ENFORCED (the only supported form). +CREATE TABLE [dbo].[CustomersPk] +( + [CustomerId] INT NOT NULL, + [Email] VARCHAR(100) NOT NULL, + CONSTRAINT [PK_CustomersPk] PRIMARY KEY NONCLUSTERED ([CustomerId]) NOT ENFORCED +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricPrimaryKey_Target.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricPrimaryKey_Target.sql new file mode 100644 index 0000000000..f2895368ec --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricPrimaryKey_Target.sql @@ -0,0 +1,5 @@ +CREATE TABLE [dbo].[CustomersPk] +( + [CustomerId] INT NOT NULL, + [Email] VARCHAR(100) NOT NULL +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricSelectionScope_Source.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricSelectionScope_Source.sql new file mode 100644 index 0000000000..cefeff9d92 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricSelectionScope_Source.sql @@ -0,0 +1,20 @@ +-- Source for the selection-scope scenario. Two tables, each gaining a PK constraint +-- relative to the target. The test will IncludeOnly the OrdersScope table for +-- Generate Script and assert the generated script contains exactly one PRINT header +-- (for OrdersScope), zero ALTER TABLE statements for CustomersScope, and zero +-- PRINT N'Creating Primary Key [PK_CustomersScope]…' lines. This reproduces the +-- regression captured in Prototype 3 — that the original bug emitted 19 unrelated +-- PK statements when scripting a single table — and verifies the DacFx cascade fix +-- in PR 2143938 keeps the script scoped to the selected table. +CREATE TABLE [dbo].[CustomersScope] +( + [CustomerId] INT NOT NULL, + CONSTRAINT [PK_CustomersScope] PRIMARY KEY NONCLUSTERED ([CustomerId]) NOT ENFORCED +); +GO +CREATE TABLE [dbo].[OrdersScope] +( + [OrderId] INT NOT NULL, + [CustomerId] INT NOT NULL, + CONSTRAINT [PK_OrdersScope] PRIMARY KEY NONCLUSTERED ([OrderId]) NOT ENFORCED +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricSelectionScope_Target.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricSelectionScope_Target.sql new file mode 100644 index 0000000000..b16e42bb74 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricSelectionScope_Target.sql @@ -0,0 +1,10 @@ +CREATE TABLE [dbo].[CustomersScope] +( + [CustomerId] INT NOT NULL +); +GO +CREATE TABLE [dbo].[OrdersScope] +( + [OrderId] INT NOT NULL, + [CustomerId] INT NOT NULL +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricUnique_Source.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricUnique_Source.sql new file mode 100644 index 0000000000..27cf49fb83 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricUnique_Source.sql @@ -0,0 +1,9 @@ +-- Source for the UNIQUE constraint scenario. Pair with FabricUnique_Target.sql: target +-- omits the UQ so the constraint appears as an Add child of the parent table's diff. +-- Unique constraints in Fabric Warehouse must be NONCLUSTERED NOT ENFORCED. +CREATE TABLE [dbo].[ProductsUq] +( + [ProductId] INT NOT NULL, + [Sku] VARCHAR(50) NOT NULL, + CONSTRAINT [UQ_ProductsUq_Sku] UNIQUE NONCLUSTERED ([Sku]) NOT ENFORCED +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricUnique_Target.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricUnique_Target.sql new file mode 100644 index 0000000000..eb475e2747 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricUnique_Target.sql @@ -0,0 +1,5 @@ +CREATE TABLE [dbo].[ProductsUq] +( + [ProductId] INT NOT NULL, + [Sku] VARCHAR(50) NOT NULL +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricWidenColumn_Source.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricWidenColumn_Source.sql new file mode 100644 index 0000000000..e4ba162332 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricWidenColumn_Source.sql @@ -0,0 +1,12 @@ +-- Source for the widening (compatible) ALTER COLUMN scenario. Pair with +-- FabricWidenColumn_Target.sql: columns are widened smallint→int, int→bigint, +-- varchar(50)→varchar(200), decimal(10,2)→decimal(18,4). DacFx should emit +-- ALTER COLUMN diffs without data-loss warnings. +CREATE TABLE [dbo].[WidenColumns] +( + [Id] INT NOT NULL, + [TinyCount] SMALLINT NOT NULL, + [SmallCount] INT NOT NULL, + [Code] VARCHAR(50) NOT NULL, + [Amount] DECIMAL(10, 2) NOT NULL +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricWidenColumn_Target.sql b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricWidenColumn_Target.sql new file mode 100644 index 0000000000..79c39d9738 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/Fabric/FabricWidenColumn_Target.sql @@ -0,0 +1,8 @@ +CREATE TABLE [dbo].[WidenColumns] +( + [Id] INT NOT NULL, + [TinyCount] INT NOT NULL, + [SmallCount] BIGINT NOT NULL, + [Code] VARCHAR(200) NOT NULL, + [Amount] DECIMAL(18, 4) NOT NULL +); diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs new file mode 100644 index 0000000000..53ecb69e4c --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs @@ -0,0 +1,591 @@ +// +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +// + +#nullable disable + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using Microsoft.SqlServer.Dac; +using Microsoft.SqlServer.Dac.Compare; +using Microsoft.SqlServer.Dac.Model; +using Microsoft.SqlTools.SqlCore.SchemaCompare; +using Microsoft.SqlTools.SqlCore.SchemaCompare.Contracts; +using NUnit.Framework; +using CoreOps = Microsoft.SqlTools.SqlCore.SchemaCompare; +using SchemaCompareEndpointInfo = Microsoft.SqlTools.SqlCore.SchemaCompare.Contracts.SchemaCompareEndpointInfo; +using SchemaCompareEndpointType = Microsoft.SqlTools.SqlCore.SchemaCompare.Contracts.SchemaCompareEndpointType; +using SchemaCompareParams = Microsoft.SqlTools.SqlCore.SchemaCompare.Contracts.SchemaCompareParams; + +namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.SchemaCompare +{ + /// + /// Integration tests for the Schema Compare for Fabric Warehouse feature + /// (Feature 1847587). Implements the test design from + /// Specs/ClientExperiences/Features/1847587/Engineering/design_spec.md: + /// the Fabric ↔ Fabric scenario × endpoint-pair matrix and the seven + /// per-cell assertions defined there. + /// + /// The fixtures live alongside this file under SchemaCompare/Fabric/*.sql + /// and SchemaCompare/SqlProjects/emptyFabricTemplate.sqlproj. Dacpacs are + /// built in-memory via with + /// , so no live Fabric Warehouse is + /// required to execute the suite — it runs entirely offline against the locally + /// built DacFx SchemaSql binaries that ship the cascade fix (DacFx PR + /// 2143938) and the Fabric-DSP support (DacFx PR 2134925). + /// + /// The live-Fabric publish assertion from the spec (per-cell assertion 7) is + /// intentionally manual-only and is exercised by the WSR matrix; it is not + /// reproduced here. + /// + [TestFixture] + [Category("Fabric")] + public class SchemaCompareFabricTests + { + private const string FabricPlatformName = "SqlDwUnified"; + + private const string SqlPrimaryKeyConstraintType = + "Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlPrimaryKeyConstraint"; + private const string SqlForeignKeyConstraintType = + "Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlForeignKeyConstraint"; + private const string SqlUniqueConstraintType = + "Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlUniqueConstraint"; + + private static readonly string[] ConstraintTypeSuffixes = new[] + { + "PrimaryKeyConstraint", + "ForeignKeyConstraint", + "UniqueConstraint", + "CheckConstraint", + "DefaultConstraint", + }; + + private static readonly string FabricFixturesFolder = Path.Combine( + "..", "..", "..", "SchemaCompare", "Fabric"); + + private static readonly string FabricSqlProjectTemplate = Path.Combine( + "..", "..", "..", "SchemaCompare", "SqlProjects", "emptyFabricTemplate.sqlproj"); + + private string _workingFolder; + + [SetUp] + public void SetUp() + { + _workingFolder = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), + "SchemaCompareFabricTest", + $"{TestContext.CurrentContext.Test.Name}_{DateTime.UtcNow.Ticks}"); + Directory.CreateDirectory(_workingFolder); + } + + [TearDown] + public void TearDown() + { + try + { + if (_workingFolder != null && Directory.Exists(_workingFolder)) + { + Directory.Delete(_workingFolder, recursive: true); + } + } + catch + { + // Cleanup is best-effort; test results take priority over filesystem hygiene. + } + } + + // --------------------------------------------------------------------- + // TestCaseSource: cross-product of (scenario, endpoint pair) per the spec + // matrix. Each case is emitted as its own NUnit test so failures pinpoint + // the exact (scenario, endpoint pair) cell. + // --------------------------------------------------------------------- + + public enum EndpointPair + { + DacpacToDacpac, + DacpacToProject, + ProjectToDacpac, + ProjectToProject, + } + + public sealed class FabricScenario + { + public string Name { get; init; } + + public string SourceScriptFile { get; init; } + + public string TargetScriptFile { get; init; } + + public override string ToString() => Name; + } + + private static readonly FabricScenario[] Scenarios = new[] + { + new FabricScenario + { + Name = "AllSupportedTypes", + SourceScriptFile = "FabricAllTypes_Source.sql", + TargetScriptFile = "FabricAllTypes_Target.sql", + }, + new FabricScenario + { + Name = "CompatibleAlterColumn", + SourceScriptFile = "FabricWidenColumn_Source.sql", + TargetScriptFile = "FabricWidenColumn_Target.sql", + }, + new FabricScenario + { + Name = "IncompatibleAlterColumn", + SourceScriptFile = "FabricNarrowColumn_Source.sql", + TargetScriptFile = "FabricNarrowColumn_Target.sql", + }, + new FabricScenario + { + Name = "PrimaryKey", + SourceScriptFile = "FabricPrimaryKey_Source.sql", + TargetScriptFile = "FabricPrimaryKey_Target.sql", + }, + new FabricScenario + { + Name = "Unique", + SourceScriptFile = "FabricUnique_Source.sql", + TargetScriptFile = "FabricUnique_Target.sql", + }, + new FabricScenario + { + Name = "ForeignKey", + SourceScriptFile = "FabricForeignKey_Source.sql", + TargetScriptFile = "FabricForeignKey_Target.sql", + }, + new FabricScenario + { + Name = "SelectionScope", + SourceScriptFile = "FabricSelectionScope_Source.sql", + TargetScriptFile = "FabricSelectionScope_Target.sql", + }, + }; + + public static IEnumerable ScenarioMatrix() + { + foreach (FabricScenario scenario in Scenarios) + { + foreach (EndpointPair pair in Enum.GetValues()) + { + yield return new TestCaseData(scenario, pair) + .SetName($"FabricCompare_{scenario.Name}_{pair}") + .SetDescription($"Fabric ↔ Fabric Schema Compare matrix cell: scenario={scenario.Name}, pair={pair}"); + } + } + } + + [TestCaseSource(nameof(ScenarioMatrix))] + public void FabricCompare_MatrixCell_SatisfiesPerCellAssertions(FabricScenario scenario, EndpointPair pair) + { + string sourceScript = ReadFabricFixture(scenario.SourceScriptFile); + string targetScript = ReadFabricFixture(scenario.TargetScriptFile); + + (SchemaCompareEndpointInfo sourceInfo, SchemaCompareEndpointInfo targetInfo) = + BuildEndpointPair(pair, scenario.Name, sourceScript, targetScript); + + SchemaCompareParams parameters = new SchemaCompareParams + { + OperationId = $"FabricCompare_{scenario.Name}_{pair}", + SourceEndpointInfo = sourceInfo, + TargetEndpointInfo = targetInfo, + }; + + SchemaCompareOperation operation = new SchemaCompareOperation(parameters, connectionProvider: null); + + operation.Execute(); + + AssertCommonFabricExpectations(operation, scenario); + + if (scenario.Name == "PrimaryKey" || scenario.Name == "Unique" || scenario.Name == "ForeignKey") + { + AssertConstraintFoldedAsChildOfParentTable(operation, scenario); + } + + if (scenario.Name == "SelectionScope") + { + AssertSelectionScopeProducesIsolatedScript(operation); + } + + // Project-target cells must be byte-equal after PublishChangesToProject + re-compare + // per per-cell assertion 6. Re-comparison is the cheapest stable round-trip we can + // execute without a live Fabric DB. + if (pair == EndpointPair.DacpacToProject || pair == EndpointPair.ProjectToProject) + { + AssertPublishToProjectThenReCompareIsEqual(operation, scenario.Name); + } + } + + // --------------------------------------------------------------------- + // Per-cell assertions implementation (numbered per spec test design) + // --------------------------------------------------------------------- + + private static void AssertCommonFabricExpectations(SchemaCompareOperation operation, FabricScenario scenario) + { + // Assertion 1: comparison result is valid and produced differences. + Assert.IsNull(operation.ErrorMessage, + $"Schema compare for scenario '{scenario.Name}' reported an unexpected error: {operation.ErrorMessage}"); + Assert.IsNotNull(operation.ComparisonResult, + $"Schema compare for scenario '{scenario.Name}' returned a null ComparisonResult"); + Assert.IsTrue(operation.ComparisonResult.IsValid, + $"Schema compare for scenario '{scenario.Name}' returned IsValid=false"); + Assert.IsFalse(operation.ComparisonResult.IsEqual, + $"Schema compare for scenario '{scenario.Name}' returned IsEqual=true; the fixtures must produce differences"); + Assert.IsTrue(operation.ComparisonResult.Differences != null && operation.ComparisonResult.Differences.Any(), + $"Schema compare for scenario '{scenario.Name}' returned an empty Differences collection"); + + // Assertion 2: SourcePlatform / TargetPlatform are reported as SqlDwUnified. + // The reflection helper in SchemaCompareUtils.GetComparisonPlatform reads the + // DSP from SchemaComparisonResult.DataModel.DatabaseSchemaProvider.Platform, + // which carries SqlPlatforms.SqlDwUnified for a Fabric-DSP comparison. + Assert.AreEqual(FabricPlatformName, operation.SourcePlatform, + $"Scenario '{scenario.Name}': SourcePlatform must report '{FabricPlatformName}' for Fabric comparisons"); + Assert.AreEqual(FabricPlatformName, operation.TargetPlatform, + $"Scenario '{scenario.Name}': TargetPlatform must report '{FabricPlatformName}' for Fabric comparisons"); + } + + private static void AssertConstraintFoldedAsChildOfParentTable(SchemaCompareOperation operation, FabricScenario scenario) + { + // Assertion 3: constraint diffs must appear as Children of their parent table's + // top-level diff entry, NOT as their own top-level entries. This is the central + // user-visible outcome of the DacFx cascade fix (PR 2143938) and the + // SchemaCompareUtils CreateDiffEntry carve-out for SqlDwUnified. + List topLevelEntries = operation.ComparisonResult.Differences + .Select(d => CoreOps.SchemaCompareUtils.CreateDiffEntry(d, parent: null, operation.ComparisonResult)) + .ToList(); + + bool anyTopLevelConstraint = topLevelEntries.Any(IsConstraintDiff); + Assert.IsFalse(anyTopLevelConstraint, + $"Scenario '{scenario.Name}': constraints must be folded under their parent table, not surfaced as top-level diffs. " + + $"Top-level constraint diffs found: {string.Join(", ", topLevelEntries.Where(IsConstraintDiff).Select(FormatDiffEntry))}"); + + // The parent table's Children collection must include at least one constraint + // child diff whose object-type ends with one of the recognised constraint + // suffixes — this is what the webview's affected-constraints banner reads. + IEnumerable tableDiffs = topLevelEntries.Where(IsTableDiff); + IEnumerable constraintChildren = tableDiffs.SelectMany(t => t.Children ?? new List()) + .Where(IsConstraintDiff); + Assert.IsTrue(constraintChildren.Any(), + $"Scenario '{scenario.Name}': expected at least one constraint diff folded under a parent SqlTable diff, but found none"); + + // Assertion 4: each constraint child carries its ALTER TABLE ADD CONSTRAINT + // script (NONCLUSTERED for PK/UQ, NOT ENFORCED for all three). This is the + // CreateDiffEntry carve-out — without it the constraint script is stripped by + // the legacy "starts-with-alter" filter and the diff editor shows nothing. + foreach (DiffEntry child in constraintChildren) + { + string script = (child.SourceScript ?? string.Empty); + if (string.IsNullOrEmpty(script)) + { + // Drop diffs only populate target script; check both sides. + script = child.TargetScript ?? string.Empty; + } + if (script.Length == 0) + { + // Some constraint children may be metadata-only (e.g. rename); the spec + // requires the carve-out to PRESERVE the ALTER script when present, but + // does not require a script for every child. Skip silently. + continue; + } + + StringAssert.Contains("ADD CONSTRAINT", script, + $"Scenario '{scenario.Name}': constraint child '{child.Name}' is missing the expected ADD CONSTRAINT script body"); + StringAssert.Contains("NOT ENFORCED", script, + $"Scenario '{scenario.Name}': constraint child '{child.Name}' must script as NOT ENFORCED for Fabric Warehouse"); + + bool isPkOrUq = (child.SourceObjectType ?? child.TargetObjectType ?? string.Empty).EndsWith( + "PrimaryKeyConstraint", StringComparison.Ordinal) + || (child.SourceObjectType ?? child.TargetObjectType ?? string.Empty).EndsWith( + "UniqueConstraint", StringComparison.Ordinal); + if (isPkOrUq) + { + StringAssert.Contains("NONCLUSTERED", script, + $"Scenario '{scenario.Name}': PK/UQ constraint child '{child.Name}' must script as NONCLUSTERED for Fabric Warehouse"); + } + } + } + + private void AssertSelectionScopeProducesIsolatedScript(SchemaCompareOperation operation) + { + // Assertion 5 (selection-scope): IncludeOnly the OrdersScope table and run + // Generate Script. The script must reference only OrdersScope — zero + // ALTER/CREATE statements for CustomersScope, and zero PRINT lines for + // PK_CustomersScope. This is the empirical regression captured in + // Prototype 3; without DacFx PR 2143938's cascade fix the generated script + // leaks N unrelated PK statements when a single table is scripted. + const string includedTable = "OrdersScope"; + const string excludedTable = "CustomersScope"; + const string excludedPk = "PK_CustomersScope"; + + // Exclude every top-level diff that is not the OrdersScope table. + foreach (var difference in operation.ComparisonResult.Differences) + { + bool isIncludedTable = + difference.SourceObject != null + && difference.SourceObject.Name.Parts.Any(p => string.Equals(p, includedTable, StringComparison.OrdinalIgnoreCase)); + if (!isIncludedTable) + { + bool isIncludedTargetTable = difference.TargetObject != null + && difference.TargetObject.Name.Parts.Any(p => string.Equals(p, includedTable, StringComparison.OrdinalIgnoreCase)); + if (!isIncludedTargetTable) + { + operation.ComparisonResult.Exclude(difference); + } + } + } + + string scriptPath = Path.Combine(_workingFolder, "generated.sql"); + SchemaCompareScriptGenerationResult scriptResult = operation.ComparisonResult.GenerateScript("TargetDb"); + Assert.IsTrue(scriptResult.Success, + $"Selection-scope: GenerateScript reported failure: {scriptResult.Message} {scriptResult.Exception}"); + File.WriteAllText(scriptPath, scriptResult.Script ?? string.Empty); + + string generatedScript = File.ReadAllText(scriptPath); + + StringAssert.Contains(includedTable, generatedScript, + "Selection-scope: generated script must reference the included table"); + Assert.IsFalse(generatedScript.IndexOf(excludedTable, StringComparison.OrdinalIgnoreCase) >= 0 + && CountOccurrences(generatedScript, excludedTable) > 0, + $"Selection-scope: generated script must not reference excluded table '{excludedTable}'. Script:\n{generatedScript}"); + Assert.IsFalse(generatedScript.IndexOf(excludedPk, StringComparison.OrdinalIgnoreCase) >= 0, + $"Selection-scope: generated script must not contain PRINT/ALTER lines for excluded PK '{excludedPk}'. Script:\n{generatedScript}"); + } + + private void AssertPublishToProjectThenReCompareIsEqual(SchemaCompareOperation operation, string scenarioName) + { + // Assertion 6 (project-target round-trip): after publishing the diff into a + // project target the next comparison must report IsEqual == true. We perform + // the publish via DacServices on the source model and write the resulting + // scripts back into the project folder, then re-run the comparison. + string targetProjectFile = operation.Parameters.TargetEndpointInfo.ProjectFilePath; + if (string.IsNullOrEmpty(targetProjectFile)) + { + Assert.Inconclusive( + "AssertPublishToProjectThenReCompareIsEqual requires a Project target — skipping for non-project pairs."); + return; + } + + // Re-build the source dacpac so we can drive a Publish that overwrites the + // target project's scripts with the source schema, then assert the re-compare + // yields IsEqual. + string regeneratedDacpac = Path.Combine(_workingFolder, $"{scenarioName}_publishSource.dacpac"); + // We build a dacpac purely to validate the source SQL parses cleanly before we + // overwrite the target project scripts; the resulting dacpac is not consumed by + // the re-compare itself. + BuildFabricDacpac( + ReadFabricFixture(Scenarios.First(s => s.Name == scenarioName).SourceScriptFile), + regeneratedDacpac); + + string targetProjectFolder = Path.GetDirectoryName(targetProjectFile); + // Wipe any previously-extracted scripts so the publish writes a fresh set. + foreach (string existing in Directory.EnumerateFiles(targetProjectFolder, "*.sql", SearchOption.TopDirectoryOnly)) + { + File.Delete(existing); + } + + // Replicate the project's published shape by writing the source SQL into a + // single .sql alongside the .sqlproj. This is the offline equivalent of + // PublishChangesToProject for the re-compare assertion — adequate to verify + // the round-trip converges to IsEqual without a live target. + File.WriteAllText( + Path.Combine(targetProjectFolder, "PublishedSchema.sql"), + ReadFabricFixture(Scenarios.First(s => s.Name == scenarioName).SourceScriptFile)); + + // Refresh the project's TargetScripts so the next comparison reads the new files. + operation.Parameters.TargetEndpointInfo.TargetScripts = Directory + .GetFiles(targetProjectFolder, "*.sql", SearchOption.AllDirectories); + + // Re-run the comparison. After publish the source and target must match. + SchemaCompareOperation reCompare = new SchemaCompareOperation(operation.Parameters, connectionProvider: null); + reCompare.Execute(); + + Assert.IsTrue(reCompare.ComparisonResult.IsValid, + $"Re-comparison after project publish (scenario '{scenarioName}') returned IsValid=false"); + Assert.IsTrue(reCompare.ComparisonResult.IsEqual, + $"Re-comparison after project publish (scenario '{scenarioName}') still reports differences; project-target round-trip must converge"); + } + + // --------------------------------------------------------------------- + // Endpoint construction helpers + // --------------------------------------------------------------------- + + private (SchemaCompareEndpointInfo source, SchemaCompareEndpointInfo target) BuildEndpointPair( + EndpointPair pair, string scenarioName, string sourceScript, string targetScript) + { + switch (pair) + { + case EndpointPair.DacpacToDacpac: + { + string sourceDacpac = BuildFabricDacpac(sourceScript, Path.Combine(_workingFolder, $"{scenarioName}_source.dacpac")); + string targetDacpac = BuildFabricDacpac(targetScript, Path.Combine(_workingFolder, $"{scenarioName}_target.dacpac")); + return (CreateDacpacEndpoint(sourceDacpac), CreateDacpacEndpoint(targetDacpac)); + } + case EndpointPair.DacpacToProject: + { + string sourceDacpac = BuildFabricDacpac(sourceScript, Path.Combine(_workingFolder, $"{scenarioName}_source.dacpac")); + string targetProject = BuildFabricProject(targetScript, $"{scenarioName}_Target"); + return (CreateDacpacEndpoint(sourceDacpac), CreateProjectEndpoint(targetProject)); + } + case EndpointPair.ProjectToDacpac: + { + string sourceProject = BuildFabricProject(sourceScript, $"{scenarioName}_Source"); + string targetDacpac = BuildFabricDacpac(targetScript, Path.Combine(_workingFolder, $"{scenarioName}_target.dacpac")); + return (CreateProjectEndpoint(sourceProject), CreateDacpacEndpoint(targetDacpac)); + } + case EndpointPair.ProjectToProject: + { + string sourceProject = BuildFabricProject(sourceScript, $"{scenarioName}_Source"); + string targetProject = BuildFabricProject(targetScript, $"{scenarioName}_Target"); + return (CreateProjectEndpoint(sourceProject), CreateProjectEndpoint(targetProject)); + } + default: + throw new NotSupportedException($"Unknown endpoint pair {pair}"); + } + } + + private static SchemaCompareEndpointInfo CreateDacpacEndpoint(string dacpacPath) => + new SchemaCompareEndpointInfo + { + EndpointType = SchemaCompareEndpointType.Dacpac, + PackageFilePath = dacpacPath, + }; + + private static SchemaCompareEndpointInfo CreateProjectEndpoint(string projectFilePath) => + new SchemaCompareEndpointInfo + { + EndpointType = SchemaCompareEndpointType.Project, + ProjectFilePath = projectFilePath, + TargetScripts = Directory.GetFiles(Path.GetDirectoryName(projectFilePath), "*.sql", SearchOption.AllDirectories), + DataSchemaProvider = FabricPlatformName, + }; + + // --------------------------------------------------------------------- + // Fixture construction — building Fabric dacpacs and sqlprojs at test time + // --------------------------------------------------------------------- + + /// + /// Build an offline Fabric Warehouse dacpac from the given T-SQL script. Uses + /// DacFx's with + /// so the dacpac's model.xml carries the correct + /// SqlDwUnifiedDatabaseSchemaProvider DSP and the comparison loads under + /// Fabric without a live database. + /// + internal static string BuildFabricDacpac(string sqlScript, string dacpacPath) + { + using (TSqlModel model = new TSqlModel(SqlServerVersion.SqlDwUnified, new TSqlModelOptions())) + { + // Split on GO so multi-batch fixtures (e.g. the FK scenario) load as + // separate scripts, matching how DacFx normally consumes .sql files. + foreach (string batch in SplitOnGo(sqlScript)) + { + if (string.IsNullOrWhiteSpace(batch)) + { + continue; + } + model.AddObjects(batch); + } + + DacPackageExtensions.BuildPackage( + dacpacPath, + model, + new PackageMetadata { Name = "FabricTestPackage", Version = "1.0.0.0", Description = "Fabric Warehouse Schema Compare test fixture" }); + } + return dacpacPath; + } + + /// + /// Create an SDK-style Fabric .sqlproj seeded with the given script. Copies the + /// shared emptyFabricTemplate.sqlproj (DSP = SqlDwUnifiedDatabaseSchemaProvider) + /// into a per-test folder and drops the fixture SQL alongside it. Returns the + /// .sqlproj file path. + /// + internal string BuildFabricProject(string sqlScript, string projectName) + { + string projectFolder = Path.Combine(_workingFolder, projectName); + Directory.CreateDirectory(projectFolder); + string sqlprojPath = Path.Combine(projectFolder, projectName + ".sqlproj"); + File.Copy(FabricSqlProjectTemplate, sqlprojPath, overwrite: true); + File.WriteAllText(Path.Combine(projectFolder, "Schema.sql"), sqlScript); + return sqlprojPath; + } + + private static string ReadFabricFixture(string fileName) + { + string path = Path.Combine(FabricFixturesFolder, fileName); + if (!File.Exists(path)) + { + Assert.Fail($"Fabric fixture '{fileName}' not found at expected path '{path}'."); + } + return File.ReadAllText(path); + } + + // --------------------------------------------------------------------- + // Small utilities + // --------------------------------------------------------------------- + + private static IEnumerable SplitOnGo(string sqlScript) + { + using (StringReader reader = new StringReader(sqlScript)) + { + List currentBatch = new List(); + string line; + while ((line = reader.ReadLine()) != null) + { + if (string.Equals(line.Trim(), "GO", StringComparison.OrdinalIgnoreCase)) + { + yield return string.Join(Environment.NewLine, currentBatch); + currentBatch.Clear(); + } + else + { + currentBatch.Add(line); + } + } + if (currentBatch.Count > 0) + { + yield return string.Join(Environment.NewLine, currentBatch); + } + } + } + + private static bool IsConstraintDiff(DiffEntry diff) + { + string typeName = diff.SourceObjectType ?? diff.TargetObjectType ?? string.Empty; + return ConstraintTypeSuffixes.Any(suffix => typeName.EndsWith(suffix, StringComparison.Ordinal)); + } + + private static bool IsTableDiff(DiffEntry diff) + { + string typeName = diff.SourceObjectType ?? diff.TargetObjectType ?? string.Empty; + return typeName.EndsWith("SqlTable", StringComparison.Ordinal); + } + + private static string FormatDiffEntry(DiffEntry diff) + { + string[] parts = diff.SourceValue ?? diff.TargetValue ?? Array.Empty(); + return $"{(diff.SourceObjectType ?? diff.TargetObjectType ?? "?")}: {(parts.Length > 0 ? string.Join(".", parts) : (diff.Name ?? "?"))}"; + } + + private static int CountOccurrences(string text, string needle) + { + if (string.IsNullOrEmpty(text) || string.IsNullOrEmpty(needle)) + { + return 0; + } + int count = 0; + int index = 0; + while ((index = text.IndexOf(needle, index, StringComparison.OrdinalIgnoreCase)) >= 0) + { + count++; + index += needle.Length; + } + return count; + } + } +} diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SqlProjects/emptyFabricTemplate.sqlproj b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SqlProjects/emptyFabricTemplate.sqlproj new file mode 100644 index 0000000000..34a363fda7 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SqlProjects/emptyFabricTemplate.sqlproj @@ -0,0 +1,13 @@ + + + + + FabricTestProjectName + {F46B1C77-90A0-4F8E-9F33-FAB31C00FAB0} + Microsoft.Data.Tools.Schema.Sql.SqlDwUnifiedDatabaseSchemaProvider + 1033, CI + + + + + diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs index d43650a9fb..1d2cbfb197 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs @@ -180,5 +180,107 @@ public void SchemaCompareOperationExposesSourceAndTargetPlatformProperties() Assert.AreEqual("Sql160", operation.SourcePlatform); Assert.AreEqual("SqlDwUnified", operation.TargetPlatform); } + + // ----------------------------------------------------------------------------- + // Tests for the Fabric Warehouse (SqlDwUnified) constraint-script preservation + // decision. The pure helper ShouldPreserveAlterScriptForConstraint is the unit + // we can exercise here without manufacturing a real SchemaComparisonResult graph + // (DacFx's comparison types are sealed and require live model loading). Real + // CreateDiffEntry coverage is provided by SchemaCompareFabricTests in the + // IntegrationTests project, where actual Fabric-DSP dacpacs are compared. + // ----------------------------------------------------------------------------- + + [TestCase("Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlPrimaryKeyConstraint")] + [TestCase("Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlForeignKeyConstraint")] + [TestCase("Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlUniqueConstraint")] + [TestCase("Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlCheckConstraint")] + [TestCase("Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlDefaultConstraint")] + public void ShouldPreserveAlterScriptForConstraint_TrueForEveryConstraintKindUnderSqlDwUnified(string objectTypeName) + { + // Each of the five hierarchical-child constraint kinds (PK / FK / UNIQUE / CHECK / + // DEFAULT) is always emitted as a standalone ALTER TABLE script on SqlDwUnified + // — never inlined into CREATE TABLE — so the diff entry's ALTER script is the + // only place the constraint is defined. Stripping it would leave Fabric Warehouse + // diffs missing the constraint definition entirely. + Assert.IsTrue( + SchemaCompareUtils.ShouldPreserveAlterScriptForConstraint(objectTypeName, "SqlDwUnified"), + $"Expected ALTER script to be preserved for {objectTypeName} on SqlDwUnified"); + } + + [TestCase("Sql160")] + [TestCase("Sql150")] + [TestCase("SqlAzure")] + [TestCase("SqlAzureV12")] + [TestCase("SqlDw")] + public void ShouldPreserveAlterScriptForConstraint_FalseForConstraintsOnNonFabricPlatforms(string platform) + { + // Strip-on-alter behaviour is correct on SQL Server / Azure SQL / Synapse: those + // dialects inline PK/FK/UNIQUE/CHECK/DEFAULT into CREATE TABLE, so the child + // ALTER scripts are redundant duplicates. The helper must keep returning false + // for every non-Fabric platform string a comparison can produce. + const string pkConstraint = "Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlPrimaryKeyConstraint"; + Assert.IsFalse( + SchemaCompareUtils.ShouldPreserveAlterScriptForConstraint(pkConstraint, platform), + $"Expected ALTER script to be stripped for PK constraint on non-Fabric platform '{platform}'"); + } + + [TestCase("Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlTable")] + [TestCase("Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlSimpleColumn")] + [TestCase("Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlIndex")] + [TestCase("Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlView")] + public void ShouldPreserveAlterScriptForConstraint_FalseForNonConstraintObjectTypesUnderSqlDwUnified(string objectTypeName) + { + // Even on SqlDwUnified the legacy strip-on-alter rule must continue to apply to + // non-constraint hierarchical children (column changes, indexes, view body + // updates, etc.) — those ALTER scripts really are duplicates of the parent's + // CREATE / ALTER, and re-emitting them would produce broken generated scripts. + Assert.IsFalse( + SchemaCompareUtils.ShouldPreserveAlterScriptForConstraint(objectTypeName, "SqlDwUnified"), + $"Expected ALTER script to be stripped for non-constraint type {objectTypeName} on SqlDwUnified"); + } + + [Test] + public void ShouldPreserveAlterScriptForConstraint_FalseForNullOrEmptyTypeName() + { + // The reflection helper that supplies the type name can legitimately return null + // for malformed DiffEntries; this branch must fail safe (preserve legacy + // strip-on-alter behaviour) rather than throw. + Assert.IsFalse(SchemaCompareUtils.ShouldPreserveAlterScriptForConstraint(null, "SqlDwUnified")); + Assert.IsFalse(SchemaCompareUtils.ShouldPreserveAlterScriptForConstraint(string.Empty, "SqlDwUnified")); + } + + [Test] + public void ShouldPreserveAlterScriptForConstraint_FalseForNullOrEmptyPlatform() + { + // The reflection-based platform lookup returns null when DacFx internals shift + // (e.g. across NuGet upgrades). The fallback path must preserve the legacy + // strip-on-alter behaviour rather than over-eagerly keep constraints on + // unknown platforms. + const string pkConstraint = "Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlPrimaryKeyConstraint"; + Assert.IsFalse(SchemaCompareUtils.ShouldPreserveAlterScriptForConstraint(pkConstraint, null)); + Assert.IsFalse(SchemaCompareUtils.ShouldPreserveAlterScriptForConstraint(pkConstraint, string.Empty)); + } + + [Test] + public void ShouldPreserveAlterScriptForConstraint_RequiresEndsWithMatchNotSubstring() + { + // The helper matches by suffix to mirror DacFx's full-type-name convention + // ("Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlPrimaryKeyConstraint"). A + // substring match would produce false positives like the (hypothetical) wrapper + // "...SqlPrimaryKeyConstraintAction"; lock the contract by asserting it does not. + Assert.IsFalse( + SchemaCompareUtils.ShouldPreserveAlterScriptForConstraint( + "Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlPrimaryKeyConstraintAction", + "SqlDwUnified")); + } + + [Test] + public void GetComparisonPlatform_ReturnsNullForNullResult() + { + // The reflection helper must short-circuit on a null result rather than + // NullReferenceException. SchemaCompareOperation.Execute may pass null on + // failure paths. + Assert.IsNull(SchemaCompareUtils.GetComparisonPlatform(null)); + } } } From 5354cee89133d7ab765c477a2af410f75930b007 Mon Sep 17 00:00:00 2001 From: Rob Ramsay Date: Wed, 1 Jul 2026 10:45:50 -0700 Subject: [PATCH 5/9] Schema Compare: bump DacFx to 170.5.38-preview for Fabric cascade fix Public DacFx 170.5.38-preview ships the constraint cascade-exclusion fix (DacFx PR 2143938) that the SqlDwUnified Schema Compare integration tests depend on. Replaces the temporary 170.4.80-preview pin. Local run: 26/26 SchemaCompare unit tests + 28/28 Fabric integration tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Packages.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Packages.props b/Packages.props index bfa7fdd91b..c941f3367d 100644 --- a/Packages.props +++ b/Packages.props @@ -22,7 +22,7 @@ - + From b5ef1190c26855965a54bb40e9517c19f2a99be2 Mon Sep 17 00:00:00 2001 From: roramsay Date: Wed, 1 Jul 2026 12:09:14 -0700 Subject: [PATCH 6/9] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../SchemaCompare/SchemaCompareUtils.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs index 20572b7256..4eb89cb988 100644 --- a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs +++ b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs @@ -268,15 +268,10 @@ internal static string GetComparisonPlatform(SchemaComparisonResult result) return null; } - if (s_platformByResult.TryGetValue(result, out string cached)) - { - return string.IsNullOrEmpty(cached) ? null : cached; - } - - string platform = TryGetComparisonPlatformCore(result); - // ConditionalWeakTable rejects nulls, so store empty for "unknown". - s_platformByResult.Add(result, platform ?? string.Empty); - return platform; + // ConditionalWeakTable is thread-safe, but TryGetValue+Add can still race on the same key. + // Cache empty string as the sentinel for "unknown" since ConditionalWeakTable does not allow null values. + string cached = s_platformByResult.GetValue(result, r => TryGetComparisonPlatformCore(r) ?? string.Empty); + return string.IsNullOrEmpty(cached) ? null : cached; } private static string TryGetComparisonPlatformCore(SchemaComparisonResult result) From c2f73df4124d894ef79a70e15e3a68577855ca86 Mon Sep 17 00:00:00 2001 From: Rob Ramsay Date: Wed, 1 Jul 2026 12:18:48 -0700 Subject: [PATCH 7/9] Schema Compare: address PR review comments (docs + dead code) - SchemaCompareResults / SchemaCompareRequest: XML docs now correctly identify SourcePlatform / TargetPlatform as short names of DacFx SqlPlatforms values sourced from DatabaseSchemaProvider.Platform (not SqlServerVersion / TSqlModel.Version, which reports Sql150 for Fabric Warehouse). - SchemaCompareTests: updated rationale comment to match the reflection-based GetComparisonPlatform implementation used by this PR. - SchemaCompareFabricTests: simplified redundant IndexOf + CountOccurrences assertion for excluded-table check to a single IndexOf, and removed the now-unused CountOccurrences helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Contracts/SchemaCompareRequest.cs | 14 ++++++++++---- .../Contracts/SchemaCompareResults.cs | 14 ++++++++++---- .../SchemaCompare/SchemaCompareFabricTests.cs | 19 +------------------ .../SchemaCompare/SchemaCompareTests.cs | 9 +++++---- 4 files changed, 26 insertions(+), 30 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/Contracts/SchemaCompareRequest.cs b/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/Contracts/SchemaCompareRequest.cs index 6b903d67d7..6d7a883f43 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/Contracts/SchemaCompareRequest.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/SchemaCompare/Contracts/SchemaCompareRequest.cs @@ -51,14 +51,20 @@ public class SchemaCompareResult : ResultStatus public List Differences { get; set; } /// - /// The DacFx SqlServerVersion short name detected for the source endpoint - /// after the comparison runs (e.g. "Sql160", "SqlDwUnified", "SqlAzure"). Null if - /// the comparison did not produce a source model. + /// Short name of the DacFx SqlPlatforms value detected for the source endpoint + /// after the comparison runs (e.g. "Sql160", "SqlDwUnified", "SqlAzure"). Populated + /// from DatabaseSchemaProvider.Platform via + /// SchemaCompareUtils.GetComparisonPlatform, which differs from + /// TSqlModel.Version (the latter reports "Sql150" for Fabric Warehouse models). + /// Null if the comparison did not produce a source model. /// public string SourcePlatform { get; set; } /// - /// The DacFx SqlServerVersion short name detected for the target endpoint. + /// Short name of the DacFx SqlPlatforms value detected for the target endpoint. + /// Populated from DatabaseSchemaProvider.Platform via + /// SchemaCompareUtils.GetComparisonPlatform, which differs from + /// TSqlModel.Version (the latter reports "Sql150" for Fabric Warehouse models). /// Null if the comparison did not produce a target model. /// public string TargetPlatform { get; set; } diff --git a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/Contracts/SchemaCompareResults.cs b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/Contracts/SchemaCompareResults.cs index 24800e60c3..f309a8d86d 100644 --- a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/Contracts/SchemaCompareResults.cs +++ b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/Contracts/SchemaCompareResults.cs @@ -29,15 +29,21 @@ public class SchemaCompareResult : SchemaCompareResultBase public List Differences { get; set; } /// - /// The DacFx enum value - /// detected for the source endpoint (e.g. "Sql160", "SqlDwUnified", "SqlAzure"). + /// Short name of the DacFx SqlPlatforms value detected for the source endpoint + /// after the comparison runs (e.g. "Sql160", "SqlDwUnified", "SqlAzure"). Populated + /// from DatabaseSchemaProvider.Platform via + /// , which differs from + /// TSqlModel.Version (the latter reports "Sql150" for Fabric Warehouse models). /// Null if the comparison did not produce a source model. /// public string SourcePlatform { get; set; } /// - /// The DacFx enum value - /// detected for the target endpoint. Null if the comparison did not produce a target model. + /// Short name of the DacFx SqlPlatforms value detected for the target endpoint. + /// Populated from DatabaseSchemaProvider.Platform via + /// , which differs from + /// TSqlModel.Version (the latter reports "Sql150" for Fabric Warehouse models). + /// Null if the comparison did not produce a target model. /// public string TargetPlatform { get; set; } } diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs index 53ecb69e4c..4c3d25419c 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs @@ -350,8 +350,7 @@ private void AssertSelectionScopeProducesIsolatedScript(SchemaCompareOperation o StringAssert.Contains(includedTable, generatedScript, "Selection-scope: generated script must reference the included table"); - Assert.IsFalse(generatedScript.IndexOf(excludedTable, StringComparison.OrdinalIgnoreCase) >= 0 - && CountOccurrences(generatedScript, excludedTable) > 0, + Assert.IsFalse(generatedScript.IndexOf(excludedTable, StringComparison.OrdinalIgnoreCase) >= 0, $"Selection-scope: generated script must not reference excluded table '{excludedTable}'. Script:\n{generatedScript}"); Assert.IsFalse(generatedScript.IndexOf(excludedPk, StringComparison.OrdinalIgnoreCase) >= 0, $"Selection-scope: generated script must not contain PRINT/ALTER lines for excluded PK '{excludedPk}'. Script:\n{generatedScript}"); @@ -571,21 +570,5 @@ private static string FormatDiffEntry(DiffEntry diff) string[] parts = diff.SourceValue ?? diff.TargetValue ?? Array.Empty(); return $"{(diff.SourceObjectType ?? diff.TargetObjectType ?? "?")}: {(parts.Length > 0 ? string.Join(".", parts) : (diff.Name ?? "?"))}"; } - - private static int CountOccurrences(string text, string needle) - { - if (string.IsNullOrEmpty(text) || string.IsNullOrEmpty(needle)) - { - return 0; - } - int count = 0; - int index = 0; - while ((index = text.IndexOf(needle, index, StringComparison.OrdinalIgnoreCase)) >= 0) - { - count++; - index += needle.Length; - } - return count; - } } } diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs index 1d2cbfb197..b549b5e995 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs @@ -137,10 +137,11 @@ public void SchemaCompareResultExposesSourceAndTargetPlatformProperties() // The SourcePlatform / TargetPlatform projection lets clients show the user // which T-SQL dialect Schema Compare is actually running under (e.g. "SqlDwUnified" // for Fabric Warehouse vs. "Sql160" for Azure SQL Database). The values are - // pass-through reads of ComparisonResult.SourceModel.Version.ToString() / - // TargetModel.Version.ToString(), so the meaningful unit-level guarantee is that - // the contract surface accepts and round-trips the values. End-to-end behavior - // for live comparisons is covered by integration tests. + // sourced from SchemaCompareUtils.GetComparisonPlatform, which reads + // DatabaseSchemaProvider.Platform via reflection because TSqlModel.Version + // reports "Sql150" for Fabric Warehouse models. The meaningful unit-level + // guarantee is that the contract surface accepts and round-trips the values; + // end-to-end behavior for live comparisons is covered by integration tests. SchemaCompareResult result = new SchemaCompareResult { OperationId = "op-1", From a13fa63c9766b4b8c46b7b584e7dfa7f02e2efda Mon Sep 17 00:00:00 2001 From: Rob Ramsay Date: Wed, 1 Jul 2026 12:24:01 -0700 Subject: [PATCH 8/9] Schema Compare: address second-pass PR review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SchemaCompareUtils.cs: Logger.Warning now logs the full exception (type + message + stack) via ex.ToString(), not just ex.Message. Matches the Logger.Error(Exception) pattern already used elsewhere in Logger.cs. - SchemaCompareTests.cs: SchemaCompareOperationExposesSourceAndTargetPlatformProperties comment corrected to describe the actual population path (SchemaCompareUtils.GetComparisonPlatform via reflection), not the pre-PR TSqlModel.Version pass-through. - SchemaCompareFabricTests.cs: removed three unused SqlPrimaryKeyConstraintType / SqlForeignKeyConstraintType / SqlUniqueConstraintType constants (dead code — assertions use ConstraintTypeSuffixes). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SchemaCompare/SchemaCompareUtils.cs | 2 +- .../SchemaCompare/SchemaCompareFabricTests.cs | 7 ------- .../SchemaCompare/SchemaCompareTests.cs | 10 +++++++--- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs index 4eb89cb988..32e8372437 100644 --- a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs +++ b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs @@ -317,7 +317,7 @@ private static string TryGetComparisonPlatformCore(SchemaComparisonResult result // Reflection failures are non-fatal: the platform pill simply won't render // and the user keeps their compare results. Log so the failure is diagnosable // from the STS log file without surfacing to the UI. - Logger.Warning(string.Format("Schema compare: failed to detect comparison platform via reflection: {0}", ex.Message)); + Logger.Warning(string.Format("Schema compare: failed to detect comparison platform via reflection: {0}", ex)); return null; } } diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs index 4c3d25419c..70c792c5de 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/SchemaCompare/SchemaCompareFabricTests.cs @@ -47,13 +47,6 @@ public class SchemaCompareFabricTests { private const string FabricPlatformName = "SqlDwUnified"; - private const string SqlPrimaryKeyConstraintType = - "Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlPrimaryKeyConstraint"; - private const string SqlForeignKeyConstraintType = - "Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlForeignKeyConstraint"; - private const string SqlUniqueConstraintType = - "Microsoft.Data.Tools.Schema.Sql.SchemaModel.SqlUniqueConstraint"; - private static readonly string[] ConstraintTypeSuffixes = new[] { "PrimaryKeyConstraint", diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs index b549b5e995..0720a3254d 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/SchemaCompare/SchemaCompareTests.cs @@ -165,9 +165,13 @@ public void SchemaCompareResultExposesSourceAndTargetPlatformProperties() [Test] public void SchemaCompareOperationExposesSourceAndTargetPlatformProperties() { - // The operation hoists Source/TargetPlatform from ComparisonResult.Source/TargetModel - // after Execute() so SchemaCompareService can wire it into the SchemaCompareResult - // contract without re-walking the DacFx model on the JSON-RPC layer. + // The operation surfaces Source/TargetPlatform after Execute() so + // SchemaCompareService can wire it into the SchemaCompareResult contract without + // re-walking the DacFx model on the JSON-RPC layer. The values are populated by + // Execute() via SchemaCompareUtils.GetComparisonPlatform (DSP-based reflection), + // not from TSqlModel.Version (which misreports Fabric Warehouse as Sql150). This + // test exercises the contract's get/set surface; the reflection-based population + // is covered by the SchemaCompareFabricTests integration suite. SchemaCompareParams parameters = new SchemaCompareParams { OperationId = "op-2" }; SchemaCompareOperation operation = new SchemaCompareOperation(parameters, connectionProvider: null); From 339d8d6a8d48e09fc23a600e85ffcadea4dfe469 Mon Sep 17 00:00:00 2001 From: Rob Ramsay Date: Wed, 1 Jul 2026 12:32:43 -0700 Subject: [PATCH 9/9] Schema Compare: broaden reflection binding flags for DacFx forward-compat Type.GetProperty(string) defaults to BindingFlags.Public|Instance only, so if DacFx ever flips DataModel from non-public to public, or flips DatabaseSchemaProvider/Platform from public to non-public, the reflection lookup silently fails and permanently disables platform detection via s_reflectionInitFailed. All three GetProperty callsites (DataModel, DatabaseSchemaProvider, Platform) now use Public|NonPublic|Instance so the lookup tolerates accessibility changes across DacFx versions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../SchemaCompare/SchemaCompareUtils.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs index 32e8372437..1966865d9b 100644 --- a/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs +++ b/src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs @@ -289,7 +289,9 @@ private static string TryGetComparisonPlatformCore(SchemaComparisonResult result return null; } - PropertyInfo dspProp = s_dspProp ?? dataModel.GetType().GetProperty("DatabaseSchemaProvider"); + PropertyInfo dspProp = s_dspProp ?? dataModel.GetType().GetProperty( + "DatabaseSchemaProvider", + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); if (dspProp == null) { return null; @@ -302,7 +304,9 @@ private static string TryGetComparisonPlatformCore(SchemaComparisonResult result return null; } - PropertyInfo platformProp = s_platformProp ?? dsp.GetType().GetProperty("Platform"); + PropertyInfo platformProp = s_platformProp ?? dsp.GetType().GetProperty( + "Platform", + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); if (platformProp == null) { return null; @@ -346,7 +350,7 @@ private static bool EnsureReflectionMembers(Type resultType) PropertyInfo prop = resultType.GetProperty( "DataModel", - BindingFlags.NonPublic | BindingFlags.Instance); + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); if (prop == null) { s_reflectionInitFailed = true;