Skip to content

Commit

Permalink
Various fixes to computed columns
Browse files Browse the repository at this point in the history 
Fixes #1942
Fixes #1946
Fixes #1947
  • Loading branch information
@roji
roji committed Aug 7, 2021
1 parent af15291 commit 169a957
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 21 deletions.
58 changes: 37 additions & 21 deletions src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,10 @@ protected override void Generate(AlterColumnOperation operation, IModel? model,
var column = model?.GetRelationalModel().FindTable(operation.Table, operation.Schema)
?.Columns.FirstOrDefault(c => c.Name == operation.Name);

if (operation.ComputedColumnSql != null)
ApplyTsVectorColumnSql(operation, model, operation.Schema, operation.Table);
ApplyTsVectorColumnSql(operation.OldColumn, model, operation.OldColumn.Schema, operation.OldColumn.Table);

if (operation.ComputedColumnSql != operation.OldColumn.ComputedColumnSql)
{
// TODO: The following will fail if the column being altered is part of an index.
// SqlServer recreates indexes, but wait to see if PostgreSQL will introduce a proper ALTER TABLE ALTER COLUMN
Expand Down Expand Up @@ -414,7 +417,9 @@ protected override void Generate(AlterColumnOperation operation, IModel? model,
DefaultValueSql = operation.DefaultValueSql,
ComputedColumnSql = operation.ComputedColumnSql,
IsFixedLength = operation.IsFixedLength,
IsStored = operation.IsStored
IsStored = operation.IsStored,
Comment = operation.Comment,
Collation = operation.Collation
};
addColumnOperation.AddAnnotations(operation.GetAnnotations());
Generate(addColumnOperation, model, builder);
Expand Down Expand Up @@ -1384,17 +1389,7 @@ protected override void ColumnDefinition(
}
}

if (operation[NpgsqlAnnotationNames.TsVectorConfig] is string tsVectorConfig)
{
var tsVectorIncludedColumns = operation[NpgsqlAnnotationNames.TsVectorProperties] as string[];
if (tsVectorIncludedColumns == null)
throw new InvalidOperationException(
$"{nameof(NpgsqlAnnotationNames.TsVectorConfig)} is present in a migration but " +
$"{nameof(NpgsqlAnnotationNames.TsVectorProperties)} is absent or empty");

operation.ComputedColumnSql = ColumnsToTsVector(tsVectorIncludedColumns, tsVectorConfig, model, schema, table);
operation.IsStored = true;
}
ApplyTsVectorColumnSql(operation, model, schema, table);

if (operation.ComputedColumnSql != null)
{
Expand Down Expand Up @@ -1431,6 +1426,27 @@ protected override void ColumnDefinition(
}
}

/// <summary>
/// Checks for a <see cref="NpgsqlAnnotationNames.TsVectorConfig"/> annotation on the given column, and if found, assigns
/// the appropriate SQL to <see cref="ColumnOperation.ComputedColumnSql"/>.
/// </summary>
protected virtual void ApplyTsVectorColumnSql(ColumnOperation column, IModel? model, string? schema, string table)
{
if (column[NpgsqlAnnotationNames.TsVectorConfig] is string tsVectorConfig)
{
var tsVectorIncludedColumns = column[NpgsqlAnnotationNames.TsVectorProperties] as string[];
if (tsVectorIncludedColumns == null)
throw new InvalidOperationException(
$"{nameof(NpgsqlAnnotationNames.TsVectorConfig)} is present in a migration but " +
$"{nameof(NpgsqlAnnotationNames.TsVectorProperties)} is absent or empty");

column.ComputedColumnSql = ColumnsToTsVector(tsVectorIncludedColumns, tsVectorConfig, model, schema, table);
column.IsStored = true;

column.RemoveAnnotation(NpgsqlAnnotationNames.TsVectorConfig);
}
}

// Note: this definition is only used for creating new identity columns, not for alterations.
protected virtual void IdentityDefinition(
ColumnOperation operation,
Expand Down Expand Up @@ -1743,6 +1759,14 @@ private string IndexColumnList(IndexColumn[] columns, string? method)

private string ColumnsToTsVector(IEnumerable<string> columns, string tsVectorConfig, IModel? model, string? schema, string table)
{
return new StringBuilder()
.Append("to_tsvector(")
.Append(_stringTypeMapping.GenerateSqlLiteral(tsVectorConfig))
.Append(", ")
.Append(string.Join(" || ' ' || ", columns.Select(GetTsVectorColumnExpression)))
.Append(")")
.ToString();

string GetTsVectorColumnExpression(string columnName)
{
var delimitedColumnName = DelimitIdentifier(columnName);
Expand All @@ -1752,14 +1776,6 @@ string GetTsVectorColumnExpression(string columnName)
? $"coalesce({delimitedColumnName}, '')"
: delimitedColumnName;
}

return new StringBuilder()
.Append("to_tsvector(")
.Append(_stringTypeMapping.GenerateSqlLiteral(tsVectorConfig))
.Append(", ")
.Append(string.Join(" || ' ' || ", columns.Select(GetTsVectorColumnExpression)))
.Append(")")
.ToString();
}

private static bool TryParseSchema(string identifier, out string name, out string? schema)
Expand Down
102 changes: 102 additions & 0 deletions test/EFCore.PG.FunctionalTests/Migrations/MigrationsNpgsqlTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1004,6 +1004,51 @@ await Test(
public override Task Alter_column_change_computed_type()
=> Assert.ThrowsAsync<NotSupportedException>(() => base.Alter_column_change_computed());

[ConditionalTheory]
[InlineData(true)]
[InlineData(false)]
[InlineData(null)]
public virtual async Task Alter_column_make_non_computed(bool? stored)
{
if (TestEnvironment.PostgresVersion.IsUnder(12))
{
// await Assert.ThrowsAsync<NotSupportedException>(() => base.Alter_column_make_non_computed());
return;
}

if (stored != true)
{
// Non-stored generated columns aren't yet supported (PG12)
// await Assert.ThrowsAsync<NotSupportedException>(() => base.Alter_column_make_non_computed(stored));
return;
}

await Test(
builder => builder.Entity(
"People", e =>
{
e.Property<int>("Id");
e.Property<int>("X");
e.Property<int>("Y");
}),
builder => builder.Entity("People").Property<int>("Sum")
.HasComputedColumnSql($"{DelimitIdentifier("X")} + {DelimitIdentifier("Y")}"),
builder => builder.Entity("People").Property<int>("Sum"),
model =>
{
var table = Assert.Single(model.Tables);
var sumColumn = Assert.Single(table.Columns, c => c.Name == "Sum");
Assert.Null(sumColumn.ComputedColumnSql);
Assert.NotEqual(true, sumColumn.IsStored);
})

AssertSql(
@"ALTER TABLE ""People"" DROP COLUMN ""Sum"";",
//
@"ALTER TABLE ""People"" ADD ""Sum"" integer NOT NULL;");

}

public override async Task Alter_column_add_comment()
{
await base.Alter_column_add_comment();
Expand Down Expand Up @@ -1473,6 +1518,63 @@ await Test(
@"ALTER TABLE ""People"" ALTER COLUMN ""Name"" TYPE text COLLATE ""C"";");
}

[Fact]
public virtual async Task Alter_column_generated_tsvector_change_config()
{
if (TestEnvironment.PostgresVersion.IsUnder(12))
return;

await Test(
builder => builder.Entity(
"Blogs", e =>
{
e.Property<string>("Title").IsRequired();
e.Property<string>("Description");
}),
builder => builder.Entity("Blogs").Property<NpgsqlTsVector>("TsVector")
.IsGeneratedTsVectorColumn("german", "Title", "Description"),
builder => builder.Entity("Blogs").Property<NpgsqlTsVector>("TsVector")
.IsGeneratedTsVectorColumn("english", "Title", "Description"),
model =>
{
var table = Assert.Single(model.Tables);
var column = Assert.Single(table.Columns, c => c.Name == "TsVector");
Assert.Equal("tsvector", column.StoreType);
Assert.Equal(@"to_tsvector('english'::regconfig, ((""Title"" || ' '::text) || COALESCE(""Description"", ''::text)))", column.ComputedColumnSql);
});

AssertSql(
@"ALTER TABLE ""Blogs"" DROP COLUMN ""TsVector"";",
//
@"ALTER TABLE ""Blogs"" ADD ""TsVector"" tsvector GENERATED ALWAYS AS (to_tsvector('english', ""Title"" || ' ' || coalesce(""Description"", ''))) STORED;");
}

[Fact]
public virtual async Task Alter_column_computed_set_collation()
{
if (TestEnvironment.PostgresVersion.IsUnder(12))
return;

await Test(
builder => builder.Entity("People", b =>
{
b.Property<string>("Name");
b.Property<string>("Name2").HasComputedColumnSql(@"""Name""", stored: true);
}),
builder => { },
builder => builder.Entity("People").Property<string>("Name2")
.UseCollation(NonDefaultCollation),
model =>
{
var computedColumn = Assert.Single(Assert.Single(model.Tables).Columns, c => c.Name == "Name2");
Assert.Equal(@"""Name""", computedColumn.ComputedColumnSql);
Assert.Equal(NonDefaultCollation, computedColumn.Collation);
});

AssertSql(
@"ALTER TABLE ""People"" ALTER COLUMN ""Name2"" TYPE text COLLATE ""POSIX"";");
}

public override async Task Drop_column()
{
await base.Drop_column();
Expand Down

0 comments on commit 169a957

Please sign in to comment.