Skip to content
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
66 changes: 66 additions & 0 deletions src/Weasel.Postgresql.Tests/Tables/TableDeltaTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,72 @@ public void invalid_if_any_new_columns_cannot_be_modified()
delta.Difference.ShouldBe(SchemaPatchDifference.Invalid);
}

// weasel#399: consumers declare "this wider actual column is acceptable" by overriding the
// virtual Equals(object) on their own TableColumn subclass — that is the seam Marten uses so
// an integer mt_version tolerates a column an earlier release already migrated to bigint
// (marten#4614/#4742). Computed-column detection (weasel#373) routed every column through
// MatchesForDelta, where a bare Equals(actual) binds to the protected, NON-virtual
// Equals(TableColumn) overload and skips the override. The column then landed in
// Columns.Different, so the table was classified Update while the writer emitted nothing —
// AssertDatabaseMatchesConfiguration threw with an empty change set.
[Fact]
public void a_subclass_equality_override_decides_whether_a_column_matches()
{
var expected = new Table("people");
expected.AddColumn<int>("id").AsPrimaryKey();
expected.AddColumn(new ToleratesWiderColumn("mt_version", "integer"));

var actual = new Table("people");
actual.AddColumn<int>("id").AsPrimaryKey();
actual.AddColumn(new TableColumn("mt_version", "bigint"));

var delta = new TableDelta(expected, actual);

delta.Columns.Matched.Select(x => x.Name).ShouldContain("mt_version");
delta.Columns.Different.ShouldBeEmpty();
delta.Difference.ShouldBe(SchemaPatchDifference.None);
}

[Fact]
public void a_subclass_equality_override_that_rejects_still_reports_a_difference()
{
var expected = new Table("people");
expected.AddColumn<int>("id").AsPrimaryKey();
expected.AddColumn(new ToleratesWiderColumn("mt_version", "integer"));

// uuid is NOT the tolerated type, so the override falls through to the base comparison
var actual = new Table("people");
actual.AddColumn<int>("id").AsPrimaryKey();
actual.AddColumn(new TableColumn("mt_version", "uuid"));

var delta = new TableDelta(expected, actual);

delta.Columns.Different.Select(x => x.Expected.Name).ShouldContain("mt_version");
}

// Mirrors Marten's RevisionColumn: an integer column that accepts an on-disk bigint rather
// than emitting a lossy narrowing cast.
public class ToleratesWiderColumn: TableColumn
{
public ToleratesWiderColumn(string name, string type): base(name, type)
{
}

public override bool Equals(object? obj)
{
if (obj is TableColumn actual
&& string.Equals(Name, actual.Name, StringComparison.OrdinalIgnoreCase)
&& actual.RawType().Equals("bigint", StringComparison.OrdinalIgnoreCase))
{
return true;
}

return base.Equals(obj);
}

public override int GetHashCode() => base.GetHashCode();
}

public class CannotAddColumn: TableColumn
{
public CannotAddColumn(string name, string type): base(name, type)
Expand Down
15 changes: 13 additions & 2 deletions src/Weasel.Postgresql/Tables/TableColumn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ internal bool MatchesForDelta(TableColumn actual, bool detectDrift)
{
// the generation expression is the column's identity — defaults and
// nullability drift don't apply to generated columns
return Equals(actual) && HasSameComputedDefinition(actual);
return equalsVirtual(actual) && HasSameComputedDefinition(actual);
}

if (actual.ComputedExpression.IsNotEmpty())
Expand All @@ -141,7 +141,7 @@ internal bool MatchesForDelta(TableColumn actual, bool detectDrift)
return true;
}

return Equals(actual) && (!detectDrift || HasSameDefaultAndNullability(actual));
return equalsVirtual(actual) && (!detectDrift || HasSameDefaultAndNullability(actual));
}

internal bool HasSameComputedDefinition(TableColumn actual)
Expand Down Expand Up @@ -172,6 +172,17 @@ internal void WriteDriftCorrections(Table parent, TableColumn actual, TextWriter
}
}

// weasel#399: MatchesForDelta must compare through the VIRTUAL Equals(object). Calling
// Equals(actual) there binds at compile time to the protected, non-virtual
// Equals(TableColumn) overload below, which silently bypasses any subclass override of
// Equals(object) — the seam consumers use to declare a wider actual column acceptable
// (Marten's integer mt_version tolerating an existing bigint, marten#4614/#4742). Before
// computed-column detection (weasel#373) the no-drift path passed a null comparer and
// ItemDelta fell back to the virtual Equals, so such columns compared equal; routing every
// comparison through MatchesForDelta changed that silently and classified the table as
// needing an update while emitting no SQL — an empty-change-set assert failure.
private bool equalsVirtual(TableColumn actual) => Equals((object)actual);

protected bool Equals(TableColumn other)
{
// Name comparison is case-insensitive: an expected case-preserved column
Expand Down
7 changes: 6 additions & 1 deletion src/Weasel.SqlServer/Tables/TableColumn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,14 @@ internal bool MatchesForDelta(TableColumn actual, bool detectDrift)
return true;
}

return Equals(actual) && (!detectDrift || HasSameDefaultAndNullability(actual));
return equalsVirtual(actual) && (!detectDrift || HasSameDefaultAndNullability(actual));
}

// weasel#399: compare through the VIRTUAL Equals(object) so a subclass override
// participates. A bare Equals(actual) binds to the protected, non-virtual
// Equals(TableColumn) overload and silently bypasses it. See the PostgreSQL twin.
private bool equalsVirtual(TableColumn actual) => Equals((object)actual);

internal bool HasSameComputedDefinition(TableColumn actual)
{
return actual.ComputedExpression.IsNotEmpty() &&
Expand Down
Loading