Custom multiline SQL migrations break due to line breaks being removed

[vertica-csv]

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

13.5.3

Bug summary

When performing migrations using the Execute.Sql("<some SQL here>").Do(); method of the MigrationBase class, the migrations may fail with a SQL error due to line breaks being removed by the Execute method in the MigrationExpressionBase class.

Specifics

The MigrationExpressionBase class splits up multi line SQL statements to ensure compatibility with SQL CE. The solution is taken from SQL Server Compact Toolbox, with the solution listed on Stack Overflow: https://stackoverflow.com/questions/13665491/sql-ce-inconsistent-with-multiple-statements :

// Stack Overflow example
using (StringReader reader = new StringReader(script))
{
	string line;
	while ((line = reader.ReadLine()) != null)
	{
		line = line.Trim();
		if (line.Equals("GO", StringComparison.OrdinalIgnoreCase))
		{
			RunCommand(sb.ToString(), dataset);
			sb.Remove(0, sb.Length);
		}
		else
		{
			sb.Append(line);
			sb.Append(Environment.NewLine); // Note the added newline
		}
	}
}

However, the solution on Stack Overflow specifically makes sure to append newlines back to the StringBuilder to account for the ones removed by the StringReader. This has been omitted in the Umbraco version of the code, resulting in the newlines being removed, causing syntax errors:

// MigrationExpressionBase.cs
var stmtBuilder = new StringBuilder();
using (var reader = new StringReader(sql))
{
	string? line;
	while ((line = reader.ReadLine()) != null)
	{
		if (line.Trim().Equals("GO", StringComparison.OrdinalIgnoreCase))
		{
			ExecuteStatement(stmtBuilder);
		}
		else
		{
			stmtBuilder.Append(line); // No new line appended, causing them to be removed
		}
	}

	if (stmtBuilder.Length > 0)
	{
		ExecuteStatement(stmtBuilder);
	}
}

Steps to reproduce

Create a new class based on MigrationBase, which uses multiline SQL statements:

Execute.Sql(
	"""
	CREATE TABLE [_DictionaryItemStatus]
	(
		[Id]           INT IDENTITY (1,1) PRIMARY KEY,
		[Key]          NVARCHAR(450)  NOT NULL UNIQUE,
		[LastModified] DATETIMEOFFSET NOT NULL
	);
	""").Do();
	
	
Execute.Sql(
	"""
	ALTER TABLE [_DictionaryItemStatus]
	ADD [LastReceivedFromTranslation] DATETIMEOFFSET NOT NULL
	CONSTRAINT [D_DictionaryItemStatus_LastReceivedTranslation] DEFAULT GETDATE()
	""").Do();

Use an implementation of INotificationHandler<UmbracoApplicationStartingNotification> to run the migration at startup, using the type name used for the MigrationBase implementation:

if (_runtimeState.Level < RuntimeLevel.Run)
{
	return;
}

var migrationPlan = new MigrationPlan("DictionaryItemStatus");

migrationPlan
	.From(string.Empty)
	.To<AddDictionaryItemStatusTable>(nameof(AddDictionaryItemStatusTable))

var upgrader = new Upgrader(migrationPlan);
upgrader.Execute(_migrationPlanExecutor, _coreScopeProvider, _keyValueService);

Expected result / actual result

Expected outcome would be Umbraco performing the migrations, then starting up normally, with a log output similar to:

[14:07:52 INF] Execute AddDictionaryItemStatusTable
[14:07:52 INF] SQL [0]: CREATE TABLE [_DictionaryItemStatus]
                        (
                            [Id] INT IDENTITY (1,1) PRIMARY KEY,
                            [Key] NVARCHAR(450) NOT NULL UNIQUE,
                            [LastModified] DATETIMEOFFSET NOT NULL
                        );
[14:07:52 INF] SQL [1]: ALTER TABLE [_DictionaryItemStatus]
                        ADD [LastReceivedFromTranslation] DATETIMEOFFSET NOT NULL
                        CONSTRAINT [D_DictionaryItemStatus_LastReceivedTranslation] DEFAULT GETDATE()

What instead happens is that Umbraco startup fails with a SQL Exception noting a syntax error about NOT NULLCONSTRAINT due to the linebreak being removed:

[14:07:52 INF] Execute AddDictionaryItemStatusTable
[14:07:52 INF] SQL [0]: CREATE TABLE [_DictionaryItemStatus](    [Id] INT IDENTITY (1,1) PRIMARY KEY,    [Key] NVARCHAR(450) NOT NULL UNIQUE,    [LastModified] DATETIMEOFFSET NOT NULL);
[14:07:52 INF] SQL [1]: ALTER TABLE [_DictionaryItemStatus]ADD [LastReceivedFromTranslation] DATETIMEOFFSET NOT NULLCONSTRAINT [D_DictionaryItemStatus_LastReceivedTranslation] DEFAULT GETDATE()
[14:07:52 ERR] Exception (0fd0b63e).

[github-actions]

Hi there @vertica-csv!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂