Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate Sqlite create table comments #17198

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 0 additions & 1 deletion EFCore.Sqlite.slnf
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"src\\EFCore.Relational\\EFCore.Relational.csproj",
"src\\EFCore.Sqlite.Core\\EFCore.Sqlite.Core.csproj",
"src\\EFCore.Sqlite.NTS\\EFCore.Sqlite.NTS.csproj",
"src\\EFCore.Sqlite\\EFCore.Sqlite.csproj",
"src\\EFCore\\EFCore.csproj",
"src\\Microsoft.Data.Sqlite.Core\\Microsoft.Data.Sqlite.Core.csproj",
"test\\EFCore.Analyzers.Tests\\EFCore.Analyzers.Test.csproj",
Expand Down
20 changes: 3 additions & 17 deletions src/EFCore.Relational/Query/QuerySqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@

using System;
using System.Collections.Generic;
using System.Data.Common;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Linq.Expressions;
using Microsoft.EntityFrameworkCore.Query.SqlExpressions;
Expand Down Expand Up @@ -72,11 +70,6 @@ public virtual IRelationalCommand GetCommand(SelectExpression selectExpression)
/// </summary>
protected virtual string AliasSeparator { get; } = " AS ";

/// <summary>
/// The default single line comment prefix.
/// </summary>
protected virtual string SingleLineCommentToken { get; } = "--";
Copy link
Contributor

Choose a reason for hiding this comment

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

@ajcvickers @smitpatel I'm breaking this (moving it to the general SQL generator so it can be shared with Migrations)


protected virtual IRelationalCommandBuilder Sql => _relationalCommandBuilder;

protected virtual void GenerateTagsHeaderComment(SelectExpression selectExpression)
Expand All @@ -85,16 +78,9 @@ protected virtual void GenerateTagsHeaderComment(SelectExpression selectExpressi
{
foreach (var tag in selectExpression.Tags)
{
using (var reader = new StringReader(tag))
{
string line;
while ((line = reader.ReadLine()) != null)
{
_relationalCommandBuilder.Append(SingleLineCommentToken).Append(" ").AppendLine(line);
}
}

_relationalCommandBuilder.AppendLine();
_relationalCommandBuilder
.AppendLines(_sqlGenerationHelper.GenerateComment(tag))
.AppendLine();
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions src/EFCore.Relational/Storage/ISqlGenerationHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ public interface ISqlGenerationHelper
/// </summary>
string BatchTerminator { get; }

/// <summary>
/// The default single-line comment prefix.
/// </summary>
string SingleLineCommentToken { get; }

/// <summary>
/// Generates a valid parameter name for the given candidate name.
/// </summary>
Expand Down Expand Up @@ -98,5 +103,12 @@ public interface ISqlGenerationHelper
/// <param name="name"> The identifier to delimit. </param>
/// <param name="schema"> The schema of the identifier. </param>
void DelimitIdentifier([NotNull] StringBuilder builder, [NotNull] string name, [CanBeNull] string schema);

/// <summary>
/// Generates a SQL comment.
/// </summary>
/// <param name="text"> The comment text. </param>
/// <returns> The generated SQL. </returns>
string GenerateComment([NotNull] string text);
}
}
28 changes: 28 additions & 0 deletions src/EFCore.Relational/Storage/RelationalSqlGenerationHelper.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.IO;
using System.Text;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Utilities;
Expand Down Expand Up @@ -43,6 +44,11 @@ public RelationalSqlGenerationHelper([NotNull] RelationalSqlGenerationHelperDepe
/// </summary>
public virtual string BatchTerminator => string.Empty;

/// <summary>
/// The default single-line comment prefix.
/// </summary>
public virtual string SingleLineCommentToken => "--";

/// <summary>
/// Generates a valid parameter name for the given candidate name.
/// </summary>
Expand Down Expand Up @@ -159,5 +165,27 @@ public virtual void DelimitIdentifier(StringBuilder builder, string name, string

DelimitIdentifier(builder, name);
}

/// <summary>
/// Generates a SQL comment.
/// </summary>
/// <param name="text"> The comment text. </param>
/// <returns> The generated SQL. </returns>
public virtual string GenerateComment(string text)
{
Check.NotEmpty(text, nameof(text));

var builder = new StringBuilder();
using (var reader = new StringReader(text))
{
string line;
while ((line = reader.ReadLine()) != null)
{
builder.Append(SingleLineCommentToken).Append(" ").AppendLine(line);
}
}

return builder.ToString();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,87 @@ protected override void Generate(
}
}

base.Generate(operation, model, builder, terminate);
if (string.IsNullOrEmpty(operation.Comment))
{
base.Generate(operation, model, builder, terminate);
}
else
{
builder
.Append("CREATE TABLE ")
.Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name, operation.Schema))
.AppendLine(" (");

using (builder.Indent())
{
builder
.AppendLines(Dependencies.SqlGenerationHelper.GenerateComment(operation.Comment))
.AppendLine();
CreateTableColumns(operation, model, builder);
CreateTableConstraints(operation, model, builder);
builder.AppendLine();
}

builder.Append(")");

if (terminate)
{
builder.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
EndStatement(builder);
}
}
}

/// <summary>
/// Generates a SQL fragment for the column definitions in an <see cref="CreateTableOperation" />.
/// </summary>
/// <param name="operation"> The operation. </param>
/// <param name="model"> The target model which may be <c>null</c> if the operations exist without a model. </param>
/// <param name="builder"> The command builder to use to add the SQL fragment. </param>
protected override void CreateTableColumns(
CreateTableOperation operation,
IModel model,
MigrationCommandListBuilder builder)
{
Check.NotNull(operation, nameof(operation));
Check.NotNull(builder, nameof(builder));

if (!operation.Columns.Any(c => !string.IsNullOrEmpty(c.Comment)))
skalpin marked this conversation as resolved.
Show resolved Hide resolved
{
base.CreateTableColumns(operation, model, builder);
}
else
{
CreateTableColumnsWithComments(operation, model, builder);
}
}

private void CreateTableColumnsWithComments(
CreateTableOperation operation,
IModel model,
MigrationCommandListBuilder builder)
{
for (var i = 0; i < operation.Columns.Count; i++)
{
var column = operation.Columns[i];

if (i > 0)
{
builder.AppendLine();
}

if (!string.IsNullOrEmpty(column.Comment))
{
builder.AppendLines(Dependencies.SqlGenerationHelper.GenerateComment(column.Comment));
}

ColumnDefinition(column, model, builder);

if (i != operation.Columns.Count - 1)
{
builder.AppendLine(",");
}
}
}

/// <summary>
Expand Down
125 changes: 125 additions & 0 deletions test/EFCore.Sqlite.FunctionalTests/SqliteMigrationSqlGeneratorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,131 @@ public virtual void CreateTableOperation_old_autoincrement_annotation()
");
}

[ConditionalFact]
public virtual void CreateTableOperation_has_comment()
{
Generate(
new CreateTableOperation
{
Name = "People",
Columns =
{
new AddColumnOperation
{
Name = "Id",
Table = "People",
ClrType = typeof(int),
IsNullable = false,
Comment = "The ID"
},
new AddColumnOperation
{
Name = "UncommentedColumn1",
Table = "People",
ClrType = typeof(string),
IsNullable = false
},
new AddColumnOperation
{
Name = "UncommentedColumn2",
Table = "People",
ClrType = typeof(string),
IsNullable = false
},
new AddColumnOperation
{
Name = "Name",
Table = "People",
ClrType = typeof(string),
IsNullable = false,
Comment = "The Name"
}
}
});

AssertSql(
@"CREATE TABLE ""People"" (
-- The ID
""Id"" INTEGER NOT NULL,

""UncommentedColumn1"" TEXT NOT NULL,

""UncommentedColumn2"" TEXT NOT NULL,

-- The Name
""Name"" TEXT NOT NULL
);
");
}

[ConditionalFact]
public virtual void CreateTableOperation_has_multi_line_comment()
{
Generate(
new CreateTableOperation
{
Name = "People",
Columns =
{
new AddColumnOperation
{
Name = "Id",
Table = "People",
ClrType = typeof(int),
IsNullable = false,
Comment = @"This is a multi-line
comment.
More information can
be found in the docs."

},
}
});

AssertSql(
@"CREATE TABLE ""People"" (
-- This is a multi-line
-- comment.
-- More information can
-- be found in the docs.
""Id"" INTEGER NOT NULL
);
");
}

[ConditionalFact]
public virtual void CreateTableOperation_has_multi_line_table_comment()
{
Generate(
new CreateTableOperation
{
Name = "People",
Comment = @"Table level comment
that continues onto another line",
Columns =
{
new AddColumnOperation
{
Name = "Id",
Table = "People",
ClrType = typeof(int),
IsNullable = false,
Comment = "My Comment"
},
}
});

AssertSql(
@"CREATE TABLE ""People"" (
-- Table level comment
-- that continues onto another line

-- My Comment
""Id"" INTEGER NOT NULL
skalpin marked this conversation as resolved.
Show resolved Hide resolved
);
");
}

public SqliteMigrationSqlGeneratorTest()
: base(SqliteTestHelpers.Instance)
{
Expand Down