TableColumn.MatchesForDelta must compare through the virtual Equals (#399) - #400
Merged
Merged
Conversation
Regression from #373. Routing every column through MatchesForDelta replaced a null comparer — under which ItemDelta fell back to the VIRTUAL Equals(object) — with a call to Equals(actual) that binds at compile time to the protected, non-virtual Equals(TableColumn) overload. Any subclass override of Equals(object) was silently bypassed. 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, instead of emitting a lossy narrowing cast (marten#4614 / #4742). With the override skipped, the column landed in Columns.Different, so the table was classified as needing an Update while the writer still emitted nothing for it — AssertDatabaseMatchesConfiguration threw with an empty change set. Bisected to 9.18.0; 9.17.0 is clean. Both the PostgreSQL and SQL Server implementations now compare through the virtual member. No behaviour change for columns that do not override Equals(object), since the base override delegates to Equals(TableColumn). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #399. Regression from #373, found while bumping Marten from Weasel 9.17.0 to 9.20.1.
MatchesForDeltacompares withEquals(actual)whereactualis statically typedTableColumn, so it binds to the protected, non-virtualEquals(TableColumn)overload rather thanpublic override bool Equals(object?). Any subclass override ofEquals(object)is silently bypassed. Before #373 the no-drift path passed anullcomparer andItemDeltafell back to the virtualEquals(object), so overrides did participate.That override is a real extension seam. Marten's
RevisionColumnuses it to declare that anintegermt_versiontolerates a column an earlier release already migrated tobigint, instead of emitting a lossyUSING mt_version::integernarrowing cast (marten#4614 / #4742). With the override skipped the column lands inColumns.Different, so the table is classifiedSchemaPatchDifference.Update— while the writer still emits nothing for it, because the consumer'sAlterColumnTypeSqlcorrectly returns empty. The result is a validation failure with an empty change set:Bisected against Marten's
Bug_4614_revision_column_int_for_IRevisionedassert-check test: 9.17.0 passes, 9.18.0 / 9.19.0 / 9.20.0 / 9.20.1 all fail.Change
Both the PostgreSQL and SQL Server
TableColumn.MatchesForDeltanow compare throughEquals((object)actual). No behaviour change for a column that does not overrideEquals(object), since the base override just delegates toEquals(TableColumn).Tests
TableDeltaTests.a_subclass_equality_override_decides_whether_a_column_matches— anintegerexpected column whose subclass accepts abigintactual must land inColumns.MatchedwithDifference == None. Verified RED before the fix.a_subclass_equality_override_that_rejects_still_reports_a_difference— an override that declines (auuidactual) still produces a difference.Local:
Weasel.Postgresql.Tests787 passed / 0 failed / 3 skipped on net10.0, three consecutive runs.Note for follow-up:
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.🤖 Generated with Claude Code