diff --git a/src/Eftdb/Generators/SqlBuilderHelper.cs b/src/Eftdb/Generators/SqlBuilderHelper.cs index aab9cf3..d0d148a 100644 --- a/src/Eftdb/Generators/SqlBuilderHelper.cs +++ b/src/Eftdb/Generators/SqlBuilderHelper.cs @@ -7,7 +7,7 @@ public class SqlBuilderHelper(string quoteString) { private readonly string quoteString = quoteString; - public static void BuildQueryString(List statements, MigrationCommandListBuilder builder, bool suppressTransaction = false) + public static void BuildQueryString(List statements, MigrationCommandListBuilder builder, bool suppressTransaction = false, bool usePerform = false) { if (statements.Count == 0) { @@ -39,13 +39,51 @@ public static void BuildQueryString(List statements, MigrationCommandLis // Build each command group foreach (List group in commandGroups) { - string command = string.Join("\n", group); + List processedGroup = usePerform + ? [.. group.Select(ReplaceSelectWithPerform)] + : group; + + string command = string.Join("\n", processedGroup); builder .Append(command) .EndCommand(suppressTransaction: suppressTransaction); } } + /// + /// Replaces a leading SELECT keyword with PERFORM for use inside PL/pgSQL blocks. + /// In PL/pgSQL (e.g., idempotent migration scripts), bare SELECT statements that return + /// results fail with "query has no destination for result data". PERFORM discards the + /// results silently and is the standard PL/pgSQL equivalent of SELECT for this purpose. + /// + internal static string ReplaceSelectWithPerform(string sql) + { + string trimmed = sql.TrimStart(); + if (trimmed.StartsWith("SELECT ", StringComparison.OrdinalIgnoreCase)) + { + int leadingWhitespaceLength = sql.Length - trimmed.Length; + return string.Concat(sql.AsSpan(0, leadingWhitespaceLength), "PERFORM", trimmed.AsSpan("SELECT".Length)); + } + + return sql; + } + + /// + /// Applies to each line of a multi-line SQL string. + /// Handles multi-statement SQL blocks where each statement starts on its own line. + /// Continuation lines (FROM, WHERE, etc.) are not affected because they don't start with SELECT. + /// + internal static string ReplaceSelectWithPerformMultiLine(string sql) + { + string[] lines = sql.Split('\n'); + for (int i = 0; i < lines.Length; i++) + { + lines[i] = ReplaceSelectWithPerform(lines[i]); + } + + return string.Join('\n', lines); + } + public static void BuildQueryString(List statements, IndentedStringBuilder builder, bool suppressTransaction = false) { if (statements.Count > 0) diff --git a/src/Eftdb/TimescaleDbMigrationsSqlGenerator.cs b/src/Eftdb/TimescaleDbMigrationsSqlGenerator.cs index 1934f87..77b6683 100644 --- a/src/Eftdb/TimescaleDbMigrationsSqlGenerator.cs +++ b/src/Eftdb/TimescaleDbMigrationsSqlGenerator.cs @@ -97,9 +97,38 @@ protected override void Generate( return; } - SqlBuilderHelper.BuildQueryString(statements, builder, suppressTransaction); + bool usePerform = Options.HasFlag(MigrationsSqlGenerationOptions.Idempotent); + SqlBuilderHelper.BuildQueryString(statements, builder, suppressTransaction, usePerform); } + + /// + /// Handles raw SQL operations from migration files (migrationBuilder.Sql calls). + /// In idempotent mode, replaces SELECT with PERFORM because the SQL is wrapped + /// in a PL/pgSQL DO block where bare SELECT fails with "query has no destination for result data". + /// Skips replacement for DDL statements (CREATE, ALTER, DROP) where SELECT is part of the syntax. + /// + protected override void Generate(SqlOperation operation, IModel? model, MigrationCommandListBuilder builder) + { + if (Options.HasFlag(MigrationsSqlGenerationOptions.Idempotent) + && !IsDdlStatement(operation.Sql)) + { + string sql = SqlBuilderHelper.ReplaceSelectWithPerformMultiLine(operation.Sql); + builder.Append(sql); + builder.EndCommand(suppressTransaction: operation.SuppressTransaction); + return; + } + + base.Generate(operation, model, builder); + } + + private static bool IsDdlStatement(string sql) + { + string trimmed = sql.TrimStart(); + return trimmed.StartsWith("CREATE ", StringComparison.OrdinalIgnoreCase) + || trimmed.StartsWith("ALTER ", StringComparison.OrdinalIgnoreCase) + || trimmed.StartsWith("DROP ", StringComparison.OrdinalIgnoreCase); + } } #pragma warning disable IDE0079 } diff --git a/tests/Eftdb.Tests/Generators/SqlBuilderHelperTests.cs b/tests/Eftdb.Tests/Generators/SqlBuilderHelperTests.cs index aaec120..15ef44d 100644 --- a/tests/Eftdb.Tests/Generators/SqlBuilderHelperTests.cs +++ b/tests/Eftdb.Tests/Generators/SqlBuilderHelperTests.cs @@ -164,6 +164,165 @@ public void BuildQueryString_MigrationCommandListBuilder_WritesNothingForEmptyLi mockBuilder.Verify(b => b.Append(It.IsAny()), Times.Never); mockBuilder.Verify(b => b.EndCommand(It.IsAny()), Times.Never); } + #region ReplaceSelectWithPerform + + [Fact] + public void ReplaceSelectWithPerform_ReplacesLeadingSelect() + { + string input = "SELECT create_hypertable('public.\"Events\"', 'CapturedAt');"; + string result = SqlBuilderHelper.ReplaceSelectWithPerform(input); + Assert.Equal("PERFORM create_hypertable('public.\"Events\"', 'CapturedAt');", result); + } + + [Fact] + public void ReplaceSelectWithPerform_PreservesLeadingWhitespace() + { + string input = " SELECT add_dimension('public.\"Events\"', by_range('sensor_id'));"; + string result = SqlBuilderHelper.ReplaceSelectWithPerform(input); + Assert.Equal(" PERFORM add_dimension('public.\"Events\"', by_range('sensor_id'));", result); + } + + [Fact] + public void ReplaceSelectWithPerform_PreservesNonSelectStatements() + { + string input = "ALTER TABLE \"public\".\"Events\" SET (timescaledb.compress = true);"; + string result = SqlBuilderHelper.ReplaceSelectWithPerform(input); + Assert.Equal(input, result); + } + + [Fact] + public void ReplaceSelectWithPerform_PreservesDoBlocks() + { + string input = "DO $$ BEGIN EXECUTE 'SELECT 1'; END $$;"; + string result = SqlBuilderHelper.ReplaceSelectWithPerform(input); + Assert.Equal(input, result); + } + + [Fact] + public void ReplaceSelectWithPerform_IsCaseInsensitive() + { + string input = "select remove_retention_policy('public.\"Events\"', if_exists => true);"; + string result = SqlBuilderHelper.ReplaceSelectWithPerform(input); + Assert.StartsWith("PERFORM", result); + } + + [Fact] + public void ReplaceSelectWithPerform_HandlesMultiLineAlterJob() + { + string input = @" + SELECT alter_job(job_id, schedule_interval => INTERVAL '2 days') + FROM timescaledb_information.jobs + WHERE proc_name = 'policy_retention' AND hypertable_schema = 'public' AND hypertable_name = 'TestTable';".Trim(); + + string result = SqlBuilderHelper.ReplaceSelectWithPerform(input); + + Assert.StartsWith("PERFORM alter_job", result); + Assert.Contains("FROM timescaledb_information.jobs", result); + Assert.DoesNotContain("SELECT", result); + } + + [Fact] + public void ReplaceSelectWithPerformMultiLine_ReplacesAllSelectStatements() + { + string input = """ + SELECT create_hypertable('public."Events"', 'Time'); + SELECT add_dimension('public."Events"', by_range('sensor_id', 100)); + SELECT alter_job(job_id, schedule_interval => INTERVAL '1 day') + FROM timescaledb_information.jobs + WHERE proc_name = 'policy_retention'; + """; + + string result = SqlBuilderHelper.ReplaceSelectWithPerformMultiLine(input); + + Assert.DoesNotContain("SELECT create_hypertable", result); + Assert.DoesNotContain("SELECT add_dimension", result); + Assert.DoesNotContain("SELECT alter_job", result); + Assert.Contains("PERFORM create_hypertable", result); + Assert.Contains("PERFORM add_dimension", result); + Assert.Contains("PERFORM alter_job", result); + Assert.Contains("FROM timescaledb_information.jobs", result); + } + + [Fact] + public void ReplaceSelectWithPerformMultiLine_PreservesDoBlocks() + { + string input = """ + SELECT create_hypertable('public."Events"', 'Time'); + DO $$ + BEGIN + EXECUTE 'SELECT enable_chunk_skipping(...)'; + END $$; + """; + + string result = SqlBuilderHelper.ReplaceSelectWithPerformMultiLine(input); + + Assert.Contains("PERFORM create_hypertable", result); + Assert.Contains("DO $$", result); + Assert.Contains("EXECUTE 'SELECT enable_chunk_skipping(...)'", result); + } + + #endregion + + #region BuildQueryString_MigrationCommandListBuilder_UsePerform + + [Fact] + public void BuildQueryString_MigrationCommandListBuilder_UsePerform_ReplacesSelectWithPerform() + { + // Arrange + List statements = ["SELECT create_hypertable('public.\"Events\"', 'Time');"]; + MigrationsSqlGeneratorDependencies dependencies = new( + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of>() + ); + + Mock mockBuilder = new(dependencies); + mockBuilder.Setup(b => b.Append(It.IsAny())).Returns(mockBuilder.Object); + mockBuilder.Setup(b => b.EndCommand(It.IsAny())).Returns(mockBuilder.Object); + + // Act + SqlBuilderHelper.BuildQueryString(statements, mockBuilder.Object, usePerform: true); + + // Assert + mockBuilder.Verify(b => b.Append(It.Is(s => s.StartsWith("PERFORM"))), Times.Once); + mockBuilder.Verify(b => b.Append(It.Is(s => s.StartsWith("SELECT"))), Times.Never); + } + + [Fact] + public void BuildQueryString_MigrationCommandListBuilder_UsePerform_False_PreservesSelect() + { + // Arrange + List statements = ["SELECT create_hypertable('public.\"Events\"', 'Time');"]; + MigrationsSqlGeneratorDependencies dependencies = new( + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of>() + ); + + Mock mockBuilder = new(dependencies); + mockBuilder.Setup(b => b.Append(It.IsAny())).Returns(mockBuilder.Object); + mockBuilder.Setup(b => b.EndCommand(It.IsAny())).Returns(mockBuilder.Object); + + // Act + SqlBuilderHelper.BuildQueryString(statements, mockBuilder.Object, usePerform: false); + + // Assert + mockBuilder.Verify(b => b.Append(It.Is(s => s.StartsWith("SELECT"))), Times.Once); + } + + #endregion } #pragma warning restore EF1001 // Internal EF Core API usage. } \ No newline at end of file diff --git a/tests/Eftdb.Tests/Generators/TimescaleDbMigrationsSqlGeneratorTests.cs b/tests/Eftdb.Tests/Generators/TimescaleDbMigrationsSqlGeneratorTests.cs new file mode 100644 index 0000000..9876942 --- /dev/null +++ b/tests/Eftdb.Tests/Generators/TimescaleDbMigrationsSqlGeneratorTests.cs @@ -0,0 +1,145 @@ +using CmdScale.EntityFrameworkCore.TimescaleDB.Operations; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Migrations.Operations; + +namespace CmdScale.EntityFrameworkCore.TimescaleDB.Tests.Generators; + +public class TimescaleDbMigrationsSqlGeneratorTests +{ + private class TestContext : DbContext + { + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseNpgsql("Host=localhost;Database=test;Username=test;Password=test") + .UseTimescaleDb(); + } + + private static string GenerateSql( + List operations, + MigrationsSqlGenerationOptions? options = null) + { + using TestContext context = new(); + IMigrationsSqlGenerator sqlGenerator = context.GetService(); + + IReadOnlyList commands = options.HasValue + ? sqlGenerator.Generate(operations, context.Model, options.Value) + : sqlGenerator.Generate(operations, context.Model); + + return string.Join("\n", commands.Select(c => c.CommandText)); + } + + #region Should_Use_Perform_For_CreateHypertable_In_Idempotent_Mode + + [Fact] + public void Should_Use_Perform_For_CreateHypertable_In_Idempotent_Mode() + { + // Arrange + CreateHypertableOperation operation = new() + { + TableName = "IdempotentHypertable", + Schema = "public", + TimeColumnName = "time" + }; + List operations = [operation]; + + // Act + string sql = GenerateSql(operations, MigrationsSqlGenerationOptions.Idempotent | MigrationsSqlGenerationOptions.Script); + + // Assert + Assert.Contains("PERFORM create_hypertable", sql); + Assert.DoesNotContain("SELECT create_hypertable", sql); + } + + #endregion + + #region Should_Use_Select_For_CreateHypertable_In_NonIdempotent_Mode + + [Fact] + public void Should_Use_Select_For_CreateHypertable_In_NonIdempotent_Mode() + { + // Arrange + CreateHypertableOperation operation = new() + { + TableName = "NonIdempotentHypertable", + Schema = "public", + TimeColumnName = "time" + }; + List operations = [operation]; + + // Act + string sql = GenerateSql(operations); + + // Assert + Assert.Contains("SELECT create_hypertable", sql); + Assert.DoesNotContain("PERFORM", sql); + } + + #endregion + + #region Should_Use_Perform_For_SqlOperation_In_Idempotent_Mode + + [Fact] + public void Should_Use_Perform_For_SqlOperation_In_Idempotent_Mode() + { + // Arrange + SqlOperation operation = new() + { + Sql = "SELECT add_retention_policy('public.\"TestTable\"', drop_after => INTERVAL '30 days');" + }; + List operations = [operation]; + + // Act + string sql = GenerateSql(operations, MigrationsSqlGenerationOptions.Idempotent | MigrationsSqlGenerationOptions.Script); + + // Assert + Assert.Contains("PERFORM add_retention_policy", sql); + Assert.DoesNotContain("SELECT add_retention", sql); + } + + #endregion + + #region Should_Preserve_Select_For_DDL_SqlOperation_In_Idempotent_Mode + + [Fact] + public void Should_Preserve_Select_For_DDL_SqlOperation_In_Idempotent_Mode() + { + // Arrange + SqlOperation operation = new() + { + Sql = "CREATE MATERIALIZED VIEW daily_agg AS\nSELECT time_bucket('1 day', time) AS bucket, avg(value) FROM metrics GROUP BY bucket;" + }; + List operations = [operation]; + + // Act + string sql = GenerateSql(operations, MigrationsSqlGenerationOptions.Idempotent | MigrationsSqlGenerationOptions.Script); + + // Assert — DDL statement passes through unchanged; SELECT inside CREATE ... AS SELECT must not be replaced + Assert.Contains("SELECT", sql); + Assert.Contains("CREATE MATERIALIZED VIEW", sql); + } + + #endregion + + #region Should_Use_Select_For_SqlOperation_In_NonIdempotent_Mode + + [Fact] + public void Should_Use_Select_For_SqlOperation_In_NonIdempotent_Mode() + { + // Arrange + SqlOperation operation = new() + { + Sql = "SELECT add_retention_policy('public.\"TestTable\"', drop_after => INTERVAL '30 days');" + }; + List operations = [operation]; + + // Act + string sql = GenerateSql(operations); + + // Assert + Assert.Contains("SELECT add_retention_policy", sql); + Assert.DoesNotContain("PERFORM", sql); + } + + #endregion +}