Skip to content

Commit

Permalink
Second wave of fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@roji
roji committed Aug 9, 2019
1 parent fada576 commit ec2f71b
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 84 deletions.
7 changes: 7 additions & 0 deletions src/EFCore.Design/Migrations/Design/CSharpMigrationOperationGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,13 @@ protected virtual void Generate([NotNull] CreateTableOperation operation, [NotNu
.Append(Code.UnknownLiteral(column.DefaultValue));
}

if (column.Comment != null)
{
builder
.Append(", comment: ")
.Append(Code.Literal(operation.Comment));
}

builder.Append(")");

using (builder.Indent())
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Migrations/MigrationBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ public virtual OperationBuilder<CreateCheckConstraintOperation> CreateCheckConst
/// <param name="constraints">
/// A delegate allowing constraints to be applied over the columns configured by the 'columns' delegate above.
/// </param>
/// <param name="comment"> A comment to been applied to the table. </param>
/// <param name="comment"> A comment to be applied to the table. </param>
/// <returns> A <see cref="CreateTableBuilder{TColumns}" /> to allow further configuration to be chained. </returns>
public virtual CreateTableBuilder<TColumns> CreateTable<TColumns>(
[NotNull] string name,
Expand Down
7 changes: 5 additions & 2 deletions src/EFCore.Relational/Migrations/Operations/Builders/ColumnsBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public ColumnsBuilder([NotNull] CreateTableOperation createTableOperation)
/// <param name="defaultValueSql"> The SQL expression to use for the column's default constraint. </param>
/// <param name="computedColumnSql"> The SQL expression to use to compute the column value. </param>
/// <param name="fixedLength"> Indicates whether or not the column is constrained to fixed-length data. </param>
/// <param name="comment"> A comment to be applied to the table. </param>
/// <returns> The same builder so that multiple calls can be chained. </returns>
public virtual OperationBuilder<AddColumnOperation> Column<T>(
[CanBeNull] string type = null,
Expand All @@ -55,7 +56,8 @@ public virtual OperationBuilder<AddColumnOperation> Column<T>(
[CanBeNull] object defaultValue = null,
[CanBeNull] string defaultValueSql = null,
[CanBeNull] string computedColumnSql = null,
bool? fixedLength = null)
bool? fixedLength = null,
[CanBeNull] string comment = null)
{
var operation = new AddColumnOperation
{
Expand All @@ -71,7 +73,8 @@ public virtual OperationBuilder<AddColumnOperation> Column<T>(
DefaultValue = defaultValue,
DefaultValueSql = defaultValueSql,
ComputedColumnSql = computedColumnSql,
IsFixedLength = fixedLength
IsFixedLength = fixedLength,
Comment = comment
};
_createTableOperation.Columns.Add(operation);

Expand Down
166 changes: 86 additions & 80 deletions src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.IO;
using System.Linq;
Expand Down Expand Up @@ -40,7 +41,6 @@ public class SqlServerMigrationsSqlGenerator : MigrationsSqlGenerator
private IReadOnlyList<MigrationOperation> _operations;
private int _variableCounter;

private const string DefaultSchema = "dbo";
/// <summary>
/// Creates a new <see cref="SqlServerMigrationsSqlGenerator"/> instance.
/// </summary>
Expand Down Expand Up @@ -138,7 +138,10 @@ protected override void Generate(

if (operation.Comment != null)
{
GenerateColumnComment(builder, model, operation.Comment, null, operation.Schema, operation.Table, operation.Name);
GenerateComment(builder, model, operation.Comment, null,
operation.Schema,
"Table", operation.Table,
"Column", operation.Name);
}

builder.EndCommand(suppressTransaction: IsMemoryOptimized(operation, model, operation.Schema, operation.Table));
Expand Down Expand Up @@ -340,7 +343,10 @@ protected override void Generate(

if (operation.OldColumn.Comment != operation.Comment)
{
GenerateColumnComment(builder, model, operation.Comment, operation.OldColumn.Comment, operation.Schema, operation.Table, operation.Name);
GenerateComment(builder, model, operation.Comment, operation.OldColumn.Comment,
operation.Schema,
"Table", operation.Table,
"Column", operation.Name);
}

if (narrowed)
Expand Down Expand Up @@ -479,12 +485,22 @@ protected override void Generate(

if (operation.Comment != null)
{
GenerateTableComment(builder, model, operation.Comment, null, operation.Schema, operation.Name);
GenerateComment(builder, model, operation.Comment, null, operation.Schema, "Table", operation.Name);
}

Console.WriteLine("BEFORE");
foreach (var c in operation.Columns)
{
Console.WriteLine($"COLUMN: {c.Name}, comment={c.Comment}");
}
foreach (var column in operation.Columns.Where(c => c.Comment != null))
{
GenerateColumnComment(builder, model, column.Comment, null, operation.Schema, operation.Name, column.Name);
Console.WriteLine("COL");
GenerateComment(builder, model, column.Comment, null,
operation.Schema,
"Table", operation.Name,
"Column", column.Name,
firstComment: false);
}

builder.EndCommand(suppressTransaction: memoryOptimized);
Expand Down Expand Up @@ -672,7 +688,7 @@ protected override void Generate(EnsureSchemaOperation operation, IModel model,
Check.NotNull(operation, nameof(operation));
Check.NotNull(builder, nameof(builder));

if (string.Equals(operation.Name, DefaultSchema, StringComparison.OrdinalIgnoreCase))
if (string.Equals(operation.Name, "dbo", StringComparison.OrdinalIgnoreCase))
{
return;
}
Expand Down Expand Up @@ -968,7 +984,7 @@ protected override void Generate(AlterTableOperation operation, IModel model, Mi

if (operation.OldTable.Comment != operation.Comment)
{
GenerateTableComment(builder, model, operation.Comment, operation.OldTable.Comment, operation.Schema, operation.Name);
GenerateComment(builder, model, operation.Comment, operation.OldTable.Comment, operation.Schema, "Table", operation.Name);
}

builder.EndCommand(suppressTransaction: IsMemoryOptimized(operation, model, operation.Schema, operation.Name));
Expand Down Expand Up @@ -1642,92 +1658,80 @@ protected virtual void CreateIndexes(

/// <summary>
/// <para>
/// Generates add and drop commands for comments on tables.
/// Generates add and drop commands for comments on columns.
/// </para>
/// </summary>
/// <param name="builder"> The command builder to use to build the commands. </param>
/// <param name="model"> The target model which may be <c>null</c> if the operations exist without a model. </param>
/// <param name="comment"> The new comment to be applied. </param>
/// <param name="oldComment"> The previous comment. </param>
/// <param name="schema"> The schema of the table. </param>
/// <param name="table"> The name of the table. </param>
protected virtual void GenerateTableComment(
/// <param name="level1Type"> The type of the level1 object (Table, Index). </param>
/// <param name="level1Name"> The name of the table or index. </param>
/// <param name="level2Type"> The type of the level2 object (Column). </param>
/// <param name="level2Name"> The name of the column. </param>
/// <param name="firstComment">
/// Indicates whether this is the first comment operation being generated in this batch.
/// Only the first operation will cause the @schema variable to be declared and set.
/// </param>
protected virtual void GenerateComment(
[NotNull] MigrationCommandListBuilder builder,
[CanBeNull] IModel model,
[CanBeNull] string comment,
[CanBeNull] string oldComment,
[CanBeNull] string schema,
[NotNull] string table)
[NotNull] string level1Type,
[NotNull] string level1Name,
[CanBeNull] string level2Type = null,
[CanBeNull] string level2Name = null,
bool firstComment = true)
{
if (comment == oldComment)
{
return;
}

schema ??= model?.GetDefaultSchema() ?? DefaultSchema;

if (oldComment != null)
{
GenerateDropExtendedProperty(builder,
"Comment",
"Schema", schema,
"Table", table);
}
var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string));

if (comment != null)
schema ??= model?.GetDefaultSchema();
if (schema == null)
{
GenerateAddExtendedProperty(builder,
"Comment", comment,
"Schema", schema,
"Table", table);
if (firstComment)
{
builder.Append("DECLARE @schema AS nvarchar(max)")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
builder.Append("SET @schema = SCHEMA_NAME()")
.AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);
}
schema = "@schema";
}
}

/// <summary>
/// <para>
/// Generates add and drop commands for comments on columns.
/// </para>
/// </summary>
/// <param name="builder"> The command builder to use to build the commands. </param>
/// <param name="model"> The target model which may be <c>null</c> if the operations exist without a model. </param>
/// <param name="comment"> The new comment to be applied. </param>
/// <param name="oldComment"> The previous comment. </param>
/// <param name="schema"> The schema of the table. </param>
/// <param name="table"> The name of the table. </param>
/// <param name="columnName"> The name of the column. </param>
protected virtual void GenerateColumnComment(
[NotNull] MigrationCommandListBuilder builder,
[CanBeNull] IModel model,
[CanBeNull] string comment,
[CanBeNull] string oldComment,
[CanBeNull] string schema,
[NotNull] string table,
[NotNull] string columnName)
{
if (comment == oldComment)
else
{
return;
schema = Literal(schema);
}

schema ??= model?.GetDefaultSchema() ?? DefaultSchema;

if (oldComment != null)
{
GenerateDropExtendedProperty(builder,
"Comment",
"Schema", schema,
"Table", table,
"Column", columnName);
GenerateDropExtendedProperty(
builder,
Literal("Comment"),
Literal("Schema"), schema,
Literal(level1Type), Literal(level1Name),
level2Type == null ? null : Literal(level2Type),
level2Type == null ? null : Literal(level2Name));
}

if (comment != null)
{
GenerateAddExtendedProperty(builder,
"Comment", comment,
"Schema", schema,
"Table", table,
"Column", columnName);
Literal("Comment"), Literal(comment),
Literal("Schema"), schema,
Literal(level1Type), Literal(level1Name),
level2Type == null ? null : Literal(level2Type),
level2Type == null ? null : Literal(level2Name));
}

string Literal(string s) => stringTypeMapping.GenerateSqlLiteral(s);
}

/// <summary>
Expand Down Expand Up @@ -1770,37 +1774,38 @@ protected virtual void GenerateAddExtendedProperty(
Check.NotNull(builder, nameof(builder));
Check.NotNull(name, nameof(name));

var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string));

builder.Append("EXEC sp_addextendedproperty @name = ").Append(stringTypeMapping.GenerateSqlLiteral(name));
builder.Append("EXEC sp_addextendedproperty @name = ").Append(name);
if (value != null)
{
builder.Append(", @value = ").Append(stringTypeMapping.GenerateSqlLiteral(value));
builder.Append(", @value = ").Append(value);
}

if (level0Type != null)
{
Debug.Assert(level0Name != null);
builder
.Append(", @level0type = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level0Type))
.Append(level0Type)
.Append(", @level0name = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level0Name));
.Append(level0Name);

if (level1Type != null)
{
Debug.Assert(level1Name != null);
builder
.Append(", @level1type = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level1Type))
.Append(level1Type)
.Append(", @level1name = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level1Name));
.Append(level1Name);

if (level2Type != null)
{
Debug.Assert(level2Name != null);
builder
.Append(", @level2type = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level2Type))
.Append(level2Type)
.Append(", @level2name = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level2Name));
.Append(level2Name);
}
}
}
Expand Down Expand Up @@ -1843,33 +1848,34 @@ protected virtual void GenerateDropExtendedProperty(
Check.NotNull(builder, nameof(builder));
Check.NotNull(name, nameof(name));

var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string));

builder.Append("EXEC sp_dropextendedproperty @name = ").Append(stringTypeMapping.GenerateSqlLiteral(name));
builder.Append("EXEC sp_dropextendedproperty @name = ").Append(name);

if (level0Type != null)
{
Debug.Assert(level0Name != null);
builder
.Append(", @level0type = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level0Type))
.Append(level0Type)
.Append(", @level0name = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level0Name));
.Append(level0Name);

if (level1Type != null)
{
Debug.Assert(level1Name != null);
builder
.Append(", @level1type = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level1Type))
.Append(level1Type)
.Append(", @level1name = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level1Name));
.Append(level1Name);

if (level2Type != null)
{
Debug.Assert(level2Name != null);
builder
.Append(", @level2type = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level2Type))
.Append(level2Type)
.Append(", @level2name = ")
.Append(stringTypeMapping.GenerateSqlLiteral(level2Name));
.Append(level2Name);
}
}
}
Expand Down
Loading

0 comments on commit ec2f71b

Please sign in to comment.