Regression (9.18.0): MatchesForDelta bypasses subclass Equals(object) overrides — empty-change-set validation failure

[jeremydmiller]

Regression introduced in 9.18.0 by the computed-column delta detection (#373). Found while bumping Marten from Weasel 9.17.0 to 9.20.1: Marten.CoreTests.Bugs.Bug_4614_revision_column_int_for_IRevisioned.existing_9x_bigint_column_for_IRevisioned_is_tolerated_not_narrowed_assert_check starts failing with

Weasel.Core.Migrations.DatabaseValidationException
    "Configuration to Schema Validation for Database 'Main' Failed! These changes detected:

    "

— a validation failure whose change set is empty. Bisected: 9.17.0 passes, 9.18.0 through 9.20.1 fail.

Cause

#373 replaced the column comparer in TableDelta:

// before
Columns = new ItemDelta<TableColumn>(expected.Columns, actual.Columns,
    expected.DetectColumnDrift
        ? (e, a) => e.Equals(a) && e.HasSameDefaultAndNullability(a)
        : null);                                    // null => ItemDelta used the VIRTUAL Equals(object)

// after
Columns = new ItemDelta<TableColumn>(expected.Columns, actual.Columns,
    (e, a) => e.MatchesForDelta(a, expected.DetectColumnDrift));

and MatchesForDelta compares with Equals(actual), where actual is statically typed TableColumn. That binds at compile time to

protected bool Equals(TableColumn other)   // non-virtual

not to public override bool Equals(object?). So any subclass override of Equals(object) is silently bypassed. Previously, with DetectColumnDrift == false, the comparer was null and ItemDelta fell back to the virtual Equals(object), so overrides did participate.

That override is a real extension seam. Marten's RevisionColumn uses it to declare that an integer mt_version tolerates a column an earlier release already migrated to bigint, rather than emitting a lossy USING mt_version::integer narrowing cast (marten#4614 / #4742). With the override skipped, the column lands in TableDelta.Columns.Different, so the table is classified SchemaPatchDifference.Update — but the writer still emits nothing for it, because the consumer's AlterColumnTypeSql correctly returns empty. Hence a validation exception with an empty change set: the two paths disagree.

The same shape exists in Weasel.SqlServer.Tables.TableColumn.MatchesForDelta.

Fix

Route the comparison through the virtual member:

private bool equalsVirtual(TableColumn actual) => Equals((object)actual);

and call that from MatchesForDelta (both the computed-column branch and the ordinary branch), in both the PostgreSQL and SQL Server implementations. No behaviour change for any column that does not override Equals(object), since the base override just delegates to Equals(TableColumn).

Tests

TableDeltaTests.a_subclass_equality_override_decides_whether_a_column_matches — an integer expected column whose subclass accepts a bigint actual must land in Columns.Matched with Difference == None. Verified RED before the fix. Plus a negative twin asserting that an override which rejects (a uuid actual) still reports a difference.

Worth considering separately: Equals(TableColumn) being protected and non-virtual makes this easy to reintroduce. A virtual, publicly overridable equality member for columns would make the seam explicit.