Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<!-- TODO: Upgrade package and use Token Credential design -->
<PackageReference Update="Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider" Version="1.1.1" />
<PackageReference Update="Microsoft.SqlServer.Management.SmoMetadataProvider" Version="181.15.0" />
<PackageReference Update="Microsoft.SqlServer.DacFx" Version="170.4.80-preview" />
<PackageReference Update="Microsoft.SqlServer.DacFx" Version="170.5.38-preview" />
Comment thread
roramsay marked this conversation as resolved.
<PackageReference Update="Microsoft.SqlPackage.Core" Version="0.1.75-preview" />
<PackageReference Update="Microsoft.SqlServer.DacFx.Projects" Version="0.6.3-preview" />
<PackageReference Update="Microsoft.Azure.Kusto.Data" Version="14.0.3" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,19 @@ public class SchemaCompareResult : ResultStatus
public bool AreEqual { get; set; }

public List<DiffEntry> Differences { get; set; }

/// <summary>
/// The DacFx <c>SqlServerVersion</c> 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.
/// </summary>
public string SourcePlatform { get; set; }

/// <summary>
/// The DacFx <c>SqlServerVersion</c> short name detected for the target endpoint.
/// Null if the comparison did not produce a target model.
/// </summary>
public string TargetPlatform { get; set; }
Comment thread
roramsay marked this conversation as resolved.
Comment thread
roramsay marked this conversation as resolved.
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,19 @@ public class SchemaCompareResult : SchemaCompareResultBase
/// List of differences found between source and target.
/// </summary>
public List<DiffEntry> Differences { get; set; }

/// <summary>
/// The DacFx <see cref="Microsoft.SqlServer.Dac.Model.SqlServerVersion"/> enum value
/// detected for the source endpoint (e.g. "Sql160", "SqlDwUnified", "SqlAzure").
/// Null if the comparison did not produce a source model.
/// </summary>
public string SourcePlatform { get; set; }

/// <summary>
/// The DacFx <see cref="Microsoft.SqlServer.Dac.Model.SqlServerVersion"/> enum value
/// detected for the target endpoint. Null if the comparison did not produce a target model.
/// </summary>
public string TargetPlatform { get; set; }
Comment thread
roramsay marked this conversation as resolved.
Comment thread
roramsay marked this conversation as resolved.
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,22 @@ public class SchemaCompareOperation : IDisposable

public List<DiffEntry> Differences;

/// <summary>
/// The platform the comparison ran under (DacFx <c>SqlPlatforms</c> enum name, e.g.
/// "Sql160", "SqlAzureV12", "SqlDwUnified"). Sourced from the comparison's
/// <c>DatabaseSchemaProvider.Platform</c> (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 <see cref="SchemaCompareUtils.GetComparisonPlatform"/>.
/// Null if the
/// platform could not be detected (e.g. the comparison was never run).
Comment on lines +45 to +46
/// </summary>
public string SourcePlatform { get; set; }

/// <summary>
/// The platform the comparison ran under. See <see cref="SourcePlatform"/>.
/// </summary>
public string TargetPlatform { get; set; }

public SchemaCompareOperation(SchemaCompareParams parameters, ISchemaCompareConnectionProvider connectionProvider)
{
Validate.IsNotNull("parameters", parameters);
Expand Down Expand Up @@ -117,6 +133,26 @@ 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).
//
// 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 = SchemaCompareUtils.GetComparisonPlatform(this.ComparisonResult);
this.SourcePlatform = comparisonPlatform;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

source and target are the same?

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
var errorsList = ComparisonResult.GetErrors().Where(x => x.MessageType.Equals(DacMessageType.Error)).Select(e => e.Message).Distinct().ToList();
Expand Down
211 changes: 209 additions & 2 deletions src/Microsoft.SqlTools.SqlCore/SchemaCompare/SchemaCompareUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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)
Expand Down Expand Up @@ -51,14 +71,23 @@ 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)
{
string sourceScript = schemaComparisonResult.GetDiffEntrySourceScript(difference);

// 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);
Comment on lines 88 to 92
}
Expand All @@ -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);
}
Expand All @@ -86,6 +115,54 @@ internal static DiffEntry CreateDiffEntry(SchemaDifference difference, DiffEntry
return diffEntry;
}

private static bool IsConstraintChildOnSqlDwUnified(DiffEntry diffEntry, SchemaComparisonResult schemaComparisonResult)
{
string typeName = diffEntry.SourceObjectType ?? diffEntry.TargetObjectType;
string platform = GetComparisonPlatform(schemaComparisonResult);
return ShouldPreserveAlterScriptForConstraint(typeName, platform);
}

/// <summary>
/// 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
/// (<see cref="ConstraintTypeNameSuffixes"/>) and the comparison is running under the
/// Fabric Warehouse DSP (<see cref="SqlDwUnifiedPlatformName"/>). 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.
/// </summary>
/// <remarks>
/// Extracted from <see cref="IsConstraintChildOnSqlDwUnified"/> so it can be unit
/// tested without manufacturing a real <see cref="SchemaComparisonResult"/> graph
/// (DacFx's comparison types are sealed with no public constructor that yields a
/// usable platform projection).
/// </remarks>
internal static bool ShouldPreserveAlterScriptForConstraint(string objectTypeName, string platform)
{
if (string.IsNullOrEmpty(objectTypeName))
{
return false;
}

bool isConstraint = false;
foreach (string suffix in ConstraintTypeNameSuffixes)
{
if (objectTypeName.EndsWith(suffix, StringComparison.Ordinal))
{
isConstraint = true;
break;
}
}
if (!isConstraint)
{
return false;
}

return string.Equals(platform, SqlDwUnifiedPlatformName, StringComparison.Ordinal);
}


internal static SchemaComparisonExcludedObjectId CreateExcludedObject(SchemaCompareObjectId sourceObj)
{
try
Expand Down Expand Up @@ -155,5 +232,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<SchemaComparisonResult, string> s_platformByResult =
new ConditionalWeakTable<SchemaComparisonResult, string>();

/// <summary>
/// Returns the comparison's DSP platform as a string (e.g. "Sql160", "SqlDwUnified")
/// by reflecting into the internal <c>SchemaCompareDataModel.DatabaseSchemaProvider</c>
/// reachable from <see cref="SchemaComparisonResult"/>. Returns <c>null</c> if any step
/// of the lookup fails or returns null. Result is cached per <c>SchemaComparisonResult</c>
/// instance so repeated calls are O(1) after the first.
/// </summary>
/// <remarks>
/// This is a reflection workaround for the lack of a public DacFx accessor exposing
/// the comparison's platform. <c>TSqlModel.Version</c> would be the natural API but
/// returns <c>Sql150</c> for Fabric Warehouse models (see
/// <c>InternalModelUtils.CalculateVersionsForPlatform</c>). If DacFx adds a public
/// <c>SchemaComparisonResult.Platform</c> property in the future, replace this with
/// the direct call.
/// </remarks>
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;
}
Comment thread
roramsay marked this conversation as resolved.

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)
Comment thread
roramsay marked this conversation as resolved.
Outdated
{
return null;
}
s_dspProp = dspProp;

object dsp = dspProp.GetValue(dataModel);
if (dsp == null)
{
return null;
}
Comment on lines +292 to +305

PropertyInfo platformProp = s_platformProp ?? dsp.GetType().GetProperty("Platform");
if (platformProp == null)
Comment thread
roramsay marked this conversation as resolved.
Outdated
{
return null;
}
s_platformProp = platformProp;

object platformValue = platformProp.GetValue(dsp);
return platformValue?.ToString();
Comment on lines +307 to +317
}
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));
Comment thread
roramsay marked this conversation as resolved.
Outdated
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);
Comment thread
roramsay marked this conversation as resolved.
Outdated
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;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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
);
Loading
Loading