diff --git a/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs b/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs index a46a10df10..f71b0eb82a 100644 --- a/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs +++ b/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs @@ -119,8 +119,10 @@ public override MethodCallCodeFragment GenerateFluentApi(IEntityType entityType, Check.NotNull(entityType, nameof(entityType)); Check.NotNull(annotation, nameof(annotation)); +#pragma warning disable 618 if (annotation.Name == NpgsqlAnnotationNames.Comment) return new MethodCallCodeFragment(nameof(NpgsqlEntityTypeBuilderExtensions.ForNpgsqlHasComment), annotation.Value); +#pragma warning restore 618 if (annotation.Name == NpgsqlAnnotationNames.UnloggedTable) return new MethodCallCodeFragment(nameof(NpgsqlEntityTypeBuilderExtensions.IsUnlogged), annotation.Value); @@ -150,8 +152,10 @@ public override MethodCallCodeFragment GenerateFluentApi(IProperty property, IAn throw new ArgumentOutOfRangeException(); } +#pragma warning disable 618 case NpgsqlAnnotationNames.Comment: return new MethodCallCodeFragment(nameof(NpgsqlPropertyBuilderExtensions.ForNpgsqlHasComment), annotation.Value); +#pragma warning restore 618 } return null; diff --git a/src/EFCore.PG/Extensions/NpgsqlEntityTypeBuilderExtensions.cs b/src/EFCore.PG/Extensions/NpgsqlEntityTypeBuilderExtensions.cs index 0520d829fc..8579f83202 100644 --- a/src/EFCore.PG/Extensions/NpgsqlEntityTypeBuilderExtensions.cs +++ b/src/EFCore.PG/Extensions/NpgsqlEntityTypeBuilderExtensions.cs @@ -130,41 +130,6 @@ public static bool ForNpgsqlCanSetStorageParameter( #endregion Storage parameters - #region Comment - - /// - /// Configures the comment set on the table when targeting Npgsql. - /// - /// The builder for the entity type being configured. - /// The name of the table. - /// The same builder instance so that multiple calls can be chained. - public static EntityTypeBuilder ForNpgsqlHasComment( - [NotNull] this EntityTypeBuilder entityTypeBuilder, - [CanBeNull] string comment) - { - Check.NotNull(entityTypeBuilder, nameof(entityTypeBuilder)); - Check.NullButNotEmpty(comment, nameof(comment)); - - entityTypeBuilder.Metadata.SetNpgsqlComment(comment); - - return entityTypeBuilder; - } - - /// - /// Configures the comment set on the table when targeting Npgsql. - /// - /// The entity type being configured. - /// The builder for the entity type being configured. - /// The name of the table. - /// The same builder instance so that multiple calls can be chained. - public static EntityTypeBuilder ForNpgsqlHasComment( - [NotNull] this EntityTypeBuilder entityTypeBuilder, - [CanBeNull] string comment) - where TEntity : class - => (EntityTypeBuilder)ForNpgsqlHasComment((EntityTypeBuilder)entityTypeBuilder, comment); - - #endregion Comment - #region Unlogged Table /// @@ -395,6 +360,39 @@ public static EntityTypeBuilder ForCockroachDbInterleaveInParent entityTypeBuilder.UseCockroachDbInterleaveInParent(parentTableType, interleavePrefix); + /// + /// Configures the comment set on the table when targeting Npgsql. + /// + /// The builder for the entity type being configured. + /// The name of the table. + /// The same builder instance so that multiple calls can be chained. + [Obsolete("Use HasComment")] + public static EntityTypeBuilder ForNpgsqlHasComment( + [NotNull] this EntityTypeBuilder entityTypeBuilder, + [CanBeNull] string comment) + { + Check.NotNull(entityTypeBuilder, nameof(entityTypeBuilder)); + Check.NullButNotEmpty(comment, nameof(comment)); + + entityTypeBuilder.Metadata.SetNpgsqlComment(comment); + + return entityTypeBuilder; + } + + /// + /// Configures the comment set on the table when targeting Npgsql. + /// + /// The entity type being configured. + /// The builder for the entity type being configured. + /// The name of the table. + /// The same builder instance so that multiple calls can be chained. + [Obsolete("Use HasComment")] + public static EntityTypeBuilder ForNpgsqlHasComment( + [NotNull] this EntityTypeBuilder entityTypeBuilder, + [CanBeNull] string comment) + where TEntity : class + => (EntityTypeBuilder)ForNpgsqlHasComment((EntityTypeBuilder)entityTypeBuilder, comment); + #endregion Obsolete } } diff --git a/src/EFCore.PG/Extensions/NpgsqlEntityTypeExtensions.cs b/src/EFCore.PG/Extensions/NpgsqlEntityTypeExtensions.cs index ee400efe27..fe75fc9907 100644 --- a/src/EFCore.PG/Extensions/NpgsqlEntityTypeExtensions.cs +++ b/src/EFCore.PG/Extensions/NpgsqlEntityTypeExtensions.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; using JetBrains.Annotations; @@ -30,12 +31,6 @@ public static void SetNpgsqlStorageParameter([NotNull] this IMutableEntityType e public static void SetNpgsqlStorageParameter([NotNull] this IConventionEntityType entityType, string parameterName, object parameterValue, bool fromDataAnnotation = false) => entityType.SetOrRemoveAnnotation(NpgsqlAnnotationNames.StorageParameterPrefix + parameterName, parameterValue, fromDataAnnotation); - public static string GetNpgsqlComment([NotNull] this IEntityType entityType) - => (string)entityType[NpgsqlAnnotationNames.Comment]; - - public static void SetNpgsqlComment([NotNull] this IMutableEntityType entityType, [CanBeNull] string comment) - => entityType.SetOrRemoveAnnotation(NpgsqlAnnotationNames.Comment, comment); - public static bool GetNpgsqlIsUnlogged([NotNull] this IEntityType entityType) => entityType[NpgsqlAnnotationNames.UnloggedTable] as bool? ?? false; @@ -47,5 +42,17 @@ public static void SetNpgsqlIsUnlogged([NotNull] this IConventionEntityType enti public static CockroachDbInterleaveInParent GetNpgsqlCockroachDbInterleaveInParent([NotNull] this IEntityType entityType) => new CockroachDbInterleaveInParent(entityType); + + #region Obsolete + + [Obsolete("Use HasComment")] + public static string GetNpgsqlComment([NotNull] this IEntityType entityType) + => (string)entityType[NpgsqlAnnotationNames.Comment]; + + [Obsolete("Use HasComment")] + public static void SetNpgsqlComment([NotNull] this IMutableEntityType entityType, [CanBeNull] string comment) + => entityType.SetOrRemoveAnnotation(NpgsqlAnnotationNames.Comment, comment); + + #endregion Obsolete } } diff --git a/src/EFCore.PG/Extensions/NpgsqlPropertyBuilderExtensions.cs b/src/EFCore.PG/Extensions/NpgsqlPropertyBuilderExtensions.cs index c0b3b8d84b..7af2c8cf5c 100644 --- a/src/EFCore.PG/Extensions/NpgsqlPropertyBuilderExtensions.cs +++ b/src/EFCore.PG/Extensions/NpgsqlPropertyBuilderExtensions.cs @@ -394,39 +394,6 @@ public static bool CanSetValueGenerationStrategy( #endregion General value generation strategy - #region Comment - - /// - /// Configures the comment set on the column when targeting Npgsql. - /// - /// The builder for the property being configured. - /// The comment of the column. - /// The same builder instance so that multiple calls can be chained. - public static PropertyBuilder ForNpgsqlHasComment( - [NotNull] this PropertyBuilder propertyBuilder, - [CanBeNull] string comment) - { - Check.NotNull(propertyBuilder, nameof(propertyBuilder)); - Check.NullButNotEmpty(comment, nameof(comment)); - - propertyBuilder.Metadata.SetNpgsqlComment(comment); - - return propertyBuilder; - } - - /// - /// Configures the comment set on the column when targeting Npgsql. - /// - /// The builder for the property being configured. - /// The comment of the column. - /// The same builder instance so that multiple calls can be chained. - public static PropertyBuilder ForNpgsqlHasComment( - [NotNull] this PropertyBuilder propertyBuilder, - [CanBeNull] string comment) - => (PropertyBuilder)ForNpgsqlHasComment((PropertyBuilder)propertyBuilder, comment); - - #endregion Comment - #region Obsolete /// @@ -572,6 +539,37 @@ public static PropertyBuilder UseNpgsqlIdentityColumn( [NotNull] this PropertyBuilder propertyBuilder) => propertyBuilder.UseIdentityColumn(); + /// + /// Configures the comment set on the column when targeting Npgsql. + /// + /// The builder for the property being configured. + /// The comment of the column. + /// The same builder instance so that multiple calls can be chained. + [Obsolete("Use HasComment")] + public static PropertyBuilder ForNpgsqlHasComment( + [NotNull] this PropertyBuilder propertyBuilder, + [CanBeNull] string comment) + { + Check.NotNull(propertyBuilder, nameof(propertyBuilder)); + Check.NullButNotEmpty(comment, nameof(comment)); + + propertyBuilder.Metadata.SetNpgsqlComment(comment); + + return propertyBuilder; + } + + /// + /// Configures the comment set on the column when targeting Npgsql. + /// + /// The builder for the property being configured. + /// The comment of the column. + /// The same builder instance so that multiple calls can be chained. + [Obsolete("Use HasComment")] + public static PropertyBuilder ForNpgsqlHasComment( + [NotNull] this PropertyBuilder propertyBuilder, + [CanBeNull] string comment) + => (PropertyBuilder)ForNpgsqlHasComment((PropertyBuilder)propertyBuilder, comment); + #endregion Obsolete } } diff --git a/src/EFCore.PG/Extensions/NpgsqlPropertyExtensions.cs b/src/EFCore.PG/Extensions/NpgsqlPropertyExtensions.cs index f5eddd8394..33868347a9 100644 --- a/src/EFCore.PG/Extensions/NpgsqlPropertyExtensions.cs +++ b/src/EFCore.PG/Extensions/NpgsqlPropertyExtensions.cs @@ -227,14 +227,16 @@ static bool IsIntegerForValueGeneration(this Type type) #endregion Value generation - #region Comment + #region Obsolete + [Obsolete("Use HasComment")] public static string GetNpgsqlComment([NotNull] this IProperty property) => (string)property[NpgsqlAnnotationNames.Comment]; + [Obsolete("Use HasComment")] public static void SetNpgsqlComment([NotNull] this IMutableProperty property, [CanBeNull] string comment) => property.SetOrRemoveAnnotation(NpgsqlAnnotationNames.Comment, comment); - #endregion Comment + #endregion Obsolete } } diff --git a/src/EFCore.PG/Metadata/Internal/NpgsqlAnnotationNames.cs b/src/EFCore.PG/Metadata/Internal/NpgsqlAnnotationNames.cs index 9cc5b41947..75f37056d8 100644 --- a/src/EFCore.PG/Metadata/Internal/NpgsqlAnnotationNames.cs +++ b/src/EFCore.PG/Metadata/Internal/NpgsqlAnnotationNames.cs @@ -21,7 +21,6 @@ public static class NpgsqlAnnotationNames public const string DatabaseTemplate = Prefix + "DatabaseTemplate"; public const string Tablespace = Prefix + "Tablespace"; public const string StorageParameterPrefix = Prefix + "StorageParameter:"; - public const string Comment = Prefix + "Comment"; public const string UnloggedTable = Prefix + "UnloggedTable"; // Database model annotations @@ -45,5 +44,8 @@ public static class NpgsqlAnnotationNames [Obsolete("Replaced by ValueGenerationStrategy.SerialColumn")] public const string ValueGeneratedOnAdd = Prefix + "ValueGeneratedOnAdd"; + + [Obsolete("Replaced by built-in EF Core support")] + public const string Comment = Prefix + "Comment"; } } diff --git a/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs b/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs index 8b2dceb7af..f8104e2bb8 100644 --- a/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs +++ b/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs @@ -18,8 +18,10 @@ public NpgsqlMigrationsAnnotationProvider([NotNull] MigrationsAnnotationProvider public override IEnumerable For(IEntityType entityType) { +#pragma warning disable 618 if (entityType.GetNpgsqlComment() is string comment) yield return new Annotation(NpgsqlAnnotationNames.Comment, comment); +#pragma warning restore 618 if (entityType.GetNpgsqlIsUnlogged()) yield return new Annotation(NpgsqlAnnotationNames.UnloggedTable, entityType.GetNpgsqlIsUnlogged()); if (entityType[CockroachDbAnnotationNames.InterleaveInParent] != null) @@ -35,8 +37,10 @@ public override IEnumerable For(IProperty property) { if (property.GetNpgsqlValueGenerationStrategy() is NpgsqlValueGenerationStrategy npgsqlValueGenerationStrategy) yield return new Annotation(NpgsqlAnnotationNames.ValueGenerationStrategy, npgsqlValueGenerationStrategy); +#pragma warning disable 618 if (property.GetNpgsqlComment() is string comment) yield return new Annotation(NpgsqlAnnotationNames.Comment, comment); +#pragma warning restore 618 } public override IEnumerable For(IIndex index) diff --git a/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs b/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs index f918442d09..30b57485b4 100644 --- a/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs +++ b/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs @@ -63,8 +63,11 @@ protected override void Generate( CreateTableOperation operation, IModel model, MigrationCommandListBuilder builder, - bool terminate) + bool terminate = true) { + if (!terminate) + throw new ArgumentException($"{nameof(CreateTableOperation)} command generation must be terminated", nameof(terminate)); + // Filter out any system columns if (operation.Columns.Any(c => IsSystemColumn(c.Name))) { @@ -80,8 +83,6 @@ protected override void Generate( operation = filteredOperation; } - #region Customized base call - builder.Append("CREATE "); if (operation[NpgsqlAnnotationNames.UnloggedTable] is bool unlogged && unlogged) @@ -101,8 +102,6 @@ protected override void Generate( builder.Append(")"); - #endregion - // CockroachDB "interleave in parent" (https://www.cockroachlabs.com/docs/stable/interleave-in-parent.html) if (operation[CockroachDbAnnotationNames.InterleaveInParent] is string) { @@ -130,48 +129,29 @@ protected override void Generate( .Append(')'); } - // Comment on the table - if (operation[NpgsqlAnnotationNames.Comment] is string comment && comment.Length > 0) - { - builder.AppendLine(';'); - - var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); + builder.AppendLine(';'); - builder - .Append("COMMENT ON TABLE ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name, operation.Schema)) - .Append(" IS ") - .Append(stringTypeMapping.GenerateSqlLiteral(comment)); - } + // Comments +#pragma warning disable 618 + var tableComment = operation.Comment ?? operation[NpgsqlAnnotationNames.Comment] as string; +#pragma warning restore 618 + if (tableComment != null) + GenerateComment(operation, model, builder, tableComment, operation.Schema, operation.Name); - // Comments on the columns - foreach (var columnOp in operation.Columns.Where(c => c[NpgsqlAnnotationNames.Comment] != null)) + foreach (var column in operation.Columns) { - var columnComment = columnOp[NpgsqlAnnotationNames.Comment]; - builder.AppendLine(';'); - - var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); - - builder - .Append("COMMENT ON COLUMN ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name, operation.Schema)) - .Append('.') - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(columnOp.Name)) - .Append(" IS ") - .Append(stringTypeMapping.GenerateSqlLiteral(columnComment)); +#pragma warning disable 618 + var columnComment = column.Comment ?? column[NpgsqlAnnotationNames.Comment] as string; +#pragma warning restore 618 + if (columnComment != null) + GenerateComment(operation, model, builder, columnComment, operation.Schema, operation.Name, column.Name); } - if (terminate) - { - builder.AppendLine(';'); - EndStatement(builder); - } + EndStatement(builder); } protected override void Generate(AlterTableOperation operation, IModel model, MigrationCommandListBuilder builder) { - var madeChanges = false; - // Storage parameters var oldStorageParameters = GetStorageParameters(operation.OldTable); var newStorageParameters = GetStorageParameters(operation); @@ -193,7 +173,6 @@ protected override void Generate(AlterTableOperation operation, IModel model, Mi .Append(")"); builder.AppendLine(';'); - madeChanges = true; } var removed = oldStorageParameters @@ -213,26 +192,18 @@ protected override void Generate(AlterTableOperation operation, IModel model, Mi .Append(")"); builder.AppendLine(';'); - madeChanges = true; } // Comment + // TODO: Unfortunately EF Core doesn't have a full OldTable on AlterTableMigrations, see + // https://github.com/aspnet/EntityFrameworkCore/issues/16798 +#pragma warning disable 618 var oldComment = operation.OldTable[NpgsqlAnnotationNames.Comment] as string; - var newComment = operation[NpgsqlAnnotationNames.Comment] as string; + var newComment = operation.Comment ?? operation[NpgsqlAnnotationNames.Comment] as string; +#pragma warning restore 618 if (oldComment != newComment) - { - var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); - - builder - .Append("COMMENT ON TABLE ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name, operation.Schema)) - .Append(" IS ") - .Append(stringTypeMapping.GenerateSqlLiteral(newComment)); - - builder.AppendLine(';'); - madeChanges = true; - } + GenerateComment(operation, model, builder, newComment, operation.Schema, operation.Name); // Unlogged table (null is equivalent to false) var oldUnlogged = operation.OldTable[NpgsqlAnnotationNames.UnloggedTable] is bool ou && ou; @@ -246,12 +217,9 @@ protected override void Generate(AlterTableOperation operation, IModel model, Mi .Append(" SET ") .Append(newUnlogged ? "UNLOGGED" : "LOGGED") .AppendLine(";"); - - madeChanges = true; } - if (madeChanges) - EndStatement(builder); + builder.EndCommand(); } protected override void Generate( @@ -271,34 +239,32 @@ protected override void Generate( AddColumnOperation operation, IModel model, MigrationCommandListBuilder builder, - bool terminate) + bool terminate = true) { + if (!terminate) + throw new ArgumentException($"{nameof(AddColumnOperation)} command generation must be terminated", nameof(terminate)); + // Never touch system columns if (IsSystemColumn(operation.Name)) return; - base.Generate(operation, model, builder, terminate: false); + builder + .Append("ALTER TABLE ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) + .Append(" ADD "); - if (operation[NpgsqlAnnotationNames.Comment] is string comment && comment.Length > 0) - { - builder.AppendLine(';'); + ColumnDefinition(operation, model, builder); - var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); + builder.AppendLine(';'); // Terminate statement but keep in same command - builder - .Append("COMMENT ON COLUMN ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) - .Append('.') - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name)) - .Append(" IS ") - .Append(stringTypeMapping.GenerateSqlLiteral(comment)); - } + // Comment +#pragma warning disable 618 + var comment = operation.Comment ?? operation[NpgsqlAnnotationNames.Comment] as string; +#pragma warning restore 618 + if (comment != null) + GenerateComment(operation, model, builder, comment, operation.Schema, operation.Table, operation.Name); - if (terminate) - { - builder.AppendLine(';'); - EndStatement(builder); - } + EndStatement(builder); } protected override void Generate(AlterColumnOperation operation, IModel model, MigrationCommandListBuilder builder) @@ -487,7 +453,6 @@ protected override void Generate(AlterColumnOperation operation, IModel model, M builder.AppendLine(';'); } - // A sequence has been created because this column was altered to be a serial. // Change the sequence's ownership. if (newSequenceName != null) @@ -503,22 +468,13 @@ protected override void Generate(AlterColumnOperation operation, IModel model, M } // Comment - var oldComment = operation.OldColumn[NpgsqlAnnotationNames.Comment] as string; - var newComment = operation[NpgsqlAnnotationNames.Comment] as string; +#pragma warning disable 618 + var oldComment = operation.OldColumn.Comment ?? operation.OldColumn[NpgsqlAnnotationNames.Comment] as string; + var newComment = operation.Comment ?? operation[NpgsqlAnnotationNames.Comment] as string; +#pragma warning restore 618 if (oldComment != newComment) - { - var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); - - builder - .Append("COMMENT ON COLUMN ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Table, operation.Schema)) - .Append('.') - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(operation.Name)) - .Append(" IS ") - .Append(stringTypeMapping.GenerateSqlLiteral(newComment)) - .AppendLine(';'); - } + GenerateComment(operation, model, builder, newComment, operation.Schema, operation.Table, operation.Name); EndStatement(builder); } @@ -1006,6 +962,53 @@ protected override void Generate(CreateSequenceOperation operation, IModel model } } + /// + /// Generates SQL to create comment extended properties on table and columns. + /// + /// The operation. + /// The target model which may be null if the operations exist without a model. + /// The command builder to use to build the commands. + /// The comment to be applied. + /// The schema of the table. + /// The name of the table. + /// The column name if comment is being applied to a column. + protected override void GenerateComment( + MigrationOperation operation, + IModel model, + MigrationCommandListBuilder builder, + [CanBeNull] string comment, + string schema, + string table, + string columnName = null) + { + Check.NotNull(operation, nameof(operation)); + Check.NotNull(builder, nameof(builder)); + Check.NotNull(table, nameof(table)); + + var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); + + if (columnName == null) + { + builder + .Append("COMMENT ON TABLE ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(table, schema)) + .Append(" IS ") + .Append(stringTypeMapping.GenerateSqlLiteral(comment)) + .AppendLine(';'); + } + else + { + builder + .Append("COMMENT ON COLUMN ") + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(table, schema)) + .Append('.') + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(columnName)) + .Append(" IS ") + .Append(stringTypeMapping.GenerateSqlLiteral(comment)) + .AppendLine(';'); + } + } + #endregion Standard migrations #region Utilities diff --git a/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs b/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs index db3466dac3..ee6465592b 100644 --- a/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs +++ b/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs @@ -222,7 +222,7 @@ ns.nspname NOT IN ('pg_catalog', 'information_schema') AND }; if (reader.GetValueOrDefault("description") is string comment) - table[NpgsqlAnnotationNames.Comment] = comment; + table.Comment = comment; tables.Add(table); } @@ -405,7 +405,7 @@ nspname NOT IN ('pg_catalog', 'information_schema') AND column.ValueGenerated = ValueGenerated.OnAdd; if (record.GetValueOrDefault("description") is string comment) - column[NpgsqlAnnotationNames.Comment] = comment; + column.Comment = comment; table.Columns.Add(column); } diff --git a/test/EFCore.PG.FunctionalTests/NpgsqlMigrationSqlGeneratorTest.cs b/test/EFCore.PG.FunctionalTests/NpgsqlMigrationSqlGeneratorTest.cs index 47bfd87a11..5e1825a303 100644 --- a/test/EFCore.PG.FunctionalTests/NpgsqlMigrationSqlGeneratorTest.cs +++ b/test/EFCore.PG.FunctionalTests/NpgsqlMigrationSqlGeneratorTest.cs @@ -1006,32 +1006,41 @@ public void AlterColumnOperation_with_system_column() #endregion - #region PostgreSQL comments + #region Comments - [Fact] - public void CreateTableOperation_with_comment() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void CreateTableOperation_with_comment(bool newCommentSupport) { - Generate( - new CreateTableOperation + var op = new CreateTableOperation + { + Name = "People", + Schema = "dbo", + Columns = { - Name = "People", - Schema = "dbo", - Columns = - { - new AddColumnOperation - { - Name = "Id", - Table = "People", - ClrType = typeof(int), - IsNullable = false - }, - }, - PrimaryKey = new AddPrimaryKeyOperation + new AddColumnOperation { - Columns = new[] { "Id" } + Name = "Id", + Table = "People", + ClrType = typeof(int), + IsNullable = false }, - [NpgsqlAnnotationNames.Comment] = "Some comment", - }); + }, + PrimaryKey = new AddPrimaryKeyOperation + { + Columns = new[] { "Id" } + } + }; + +#pragma warning disable 618 + if (newCommentSupport) + op.Comment = "Some comment"; + else + op[NpgsqlAnnotationNames.Comment] = "Some comment"; +#pragma warning restore 618 + + Generate(op); Assert.Equal( "CREATE TABLE dbo.\"People\" (" + EOL + @@ -1042,25 +1051,32 @@ public void CreateTableOperation_with_comment() Sql); } - [Fact] - public void CreateTableOperation_with_comment_on_column() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void CreateTableOperation_with_comment_on_column(bool newCommentSupport) { + var addColumnOp = new AddColumnOperation + { + Name = "Id", + Table = "People", + ClrType = typeof(int), + IsNullable = false + }; + +#pragma warning disable 618 + if (newCommentSupport) + addColumnOp.Comment = "Some comment"; + else + addColumnOp[NpgsqlAnnotationNames.Comment] = "Some comment"; +#pragma warning restore 618 + Generate( new CreateTableOperation { Name = "People", Schema = "dbo", - Columns = - { - new AddColumnOperation - { - Name = "Id", - Table = "People", - ClrType = typeof(int), - IsNullable = false, - [NpgsqlAnnotationNames.Comment] = "Some comment", - } - }, + Columns = { addColumnOp }, PrimaryKey = new AddPrimaryKeyOperation { Columns = new[] { "Id" } @@ -1076,42 +1092,74 @@ public void CreateTableOperation_with_comment_on_column() Sql); } - [Fact] - public void AlterTable_change_comment() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void AlterTable_change_comment(bool newCommentSupport) { - Generate( - new AlterTableOperation - { - Name = "People", - Schema = "dbo", - OldTable = new Annotatable { [NpgsqlAnnotationNames.Comment] = "Old comment" }, - [NpgsqlAnnotationNames.Comment] = "New comment" - }); + var op = new AlterTableOperation + { + Name = "People", + Schema = "dbo", + OldTable = new Annotatable() + }; + +#pragma warning disable 618 + if (newCommentSupport) + { + //op.OldTable.Comment = "Old comment"; + op[NpgsqlAnnotationNames.Comment] = "New comment"; + return; // TODO: https://github.com/aspnet/EntityFrameworkCore/issues/16798 + } + else + { + op.OldTable[NpgsqlAnnotationNames.Comment] = "Old comment"; + op.Comment = "New comment"; + } +#pragma warning restore 618 + + Generate(op); Assert.Equal( "COMMENT ON TABLE dbo.\"People\" IS 'New comment';" + EOL, Sql); } - [Fact] - public void AlterTable_remove_comment() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void AlterTable_remove_comment(bool newCommentSupport) { - Generate( - new AlterTableOperation - { - Name = "People", - Schema = "dbo", - OldTable = new Annotatable { [NpgsqlAnnotationNames.Comment] = "New comment" } - }); + var op = new AlterTableOperation + { + Name = "People", + Schema = "dbo", + OldTable = new Annotatable() + }; + +#pragma warning disable 618 + if (newCommentSupport) + { + //op.OldTable.Comment = "Old comment"; + return; // TODO: https://github.com/aspnet/EntityFrameworkCore/issues/16798 + } + else + op.OldTable[NpgsqlAnnotationNames.Comment] = "Old comment"; +#pragma warning restore 618 + + Generate(op); + Assert.Equal( "COMMENT ON TABLE dbo.\"People\" IS NULL;" + EOL, Sql); } - [Fact] - public void AddColumnOperation_with_comment() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void AddColumnOperation_with_comment(bool newCommentSupport) { - Generate(new AddColumnOperation + var op = new AddColumnOperation { Schema = "dbo", Table = "People", @@ -1119,8 +1167,16 @@ public void AddColumnOperation_with_comment() ClrType = typeof(int), ColumnType = "int", IsNullable = false, - [NpgsqlAnnotationNames.Comment] = "Some comment", - }); + }; + +#pragma warning disable 618 + if (newCommentSupport) + op.Comment = "Some comment"; + else + op[NpgsqlAnnotationNames.Comment] = "Some comment"; +#pragma warning restore 618 + + Generate(op); Assert.Equal( "ALTER TABLE dbo.\"People\" ADD foo int NOT NULL;" + EOL + @@ -1128,22 +1184,37 @@ public void AddColumnOperation_with_comment() Sql); } - [Fact] - public void AlterColumn_change_comment() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void AlterColumn_change_comment(bool newCommentSupport) { - Generate( - new AlterColumnOperation - { - Table = "People", - Schema = "dbo", - Name = "LuckyNumber", - ClrType = typeof(int), - ColumnType = "int", - IsNullable = false, - DefaultValue = 7, - OldColumn = new ColumnOperation { [NpgsqlAnnotationNames.Comment] = "Old comment" }, - [NpgsqlAnnotationNames.Comment] = "New comment" - }); + var op = new AlterColumnOperation + { + Table = "People", + Schema = "dbo", + Name = "LuckyNumber", + ClrType = typeof(int), + ColumnType = "int", + IsNullable = false, + DefaultValue = 7, + OldColumn = new ColumnOperation(), + }; + +#pragma warning disable 618 + if (newCommentSupport) + { + op.OldColumn.Comment = "Old comment"; + op.Comment = "New comment"; + } + else + { + op.OldColumn[NpgsqlAnnotationNames.Comment] = "Old comment"; + op[NpgsqlAnnotationNames.Comment] = "New comment"; + } +#pragma warning restore 618 + + Generate(op); Assert.Equal( @"ALTER TABLE dbo.""People"" ALTER COLUMN ""LuckyNumber"" TYPE int;" + EOL + @@ -1153,21 +1224,35 @@ public void AlterColumn_change_comment() Sql); } - [Fact] - public void AlterColumn_remove_comment() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void AlterColumn_remove_comment(bool newCommentSupport) { - Generate( - new AlterColumnOperation - { - Table = "People", - Schema = "dbo", - Name = "LuckyNumber", - ClrType = typeof(int), - ColumnType = "int", - IsNullable = false, - DefaultValue = 7, - OldColumn = new ColumnOperation { [NpgsqlAnnotationNames.Comment] = "Old comment" } - }); + var op = new AlterColumnOperation + { + Table = "People", + Schema = "dbo", + Name = "LuckyNumber", + ClrType = typeof(int), + ColumnType = "int", + IsNullable = false, + DefaultValue = 7, + OldColumn = new ColumnOperation() + }; + +#pragma warning disable 618 + if (newCommentSupport) + { + op.OldColumn.Comment = "Old comment"; + } + else + { + op.OldColumn[NpgsqlAnnotationNames.Comment] = "Old comment"; + } +#pragma warning restore 618 + + Generate(op); Assert.Equal( @"ALTER TABLE dbo.""People"" ALTER COLUMN ""LuckyNumber"" TYPE int;" + EOL + diff --git a/test/EFCore.PG.FunctionalTests/Scaffolding/NpgsqlDatabaseModelFactoryTest.cs b/test/EFCore.PG.FunctionalTests/Scaffolding/NpgsqlDatabaseModelFactoryTest.cs index 539ce37a67..28bd0c8393 100644 --- a/test/EFCore.PG.FunctionalTests/Scaffolding/NpgsqlDatabaseModelFactoryTest.cs +++ b/test/EFCore.PG.FunctionalTests/Scaffolding/NpgsqlDatabaseModelFactoryTest.cs @@ -1651,8 +1651,8 @@ public void Comments() dbModel => { var table = dbModel.Tables.Single(); - Assert.Equal("table comment", table.FindAnnotation(NpgsqlAnnotationNames.Comment).Value); - Assert.Equal("column comment", table.Columns.Single().FindAnnotation(NpgsqlAnnotationNames.Comment).Value); + Assert.Equal("table comment", table.Comment); + Assert.Equal("column comment", table.Columns.Single().Comment); }, "DROP TABLE comment");