storage: version the PostgreSQL oid source cast per export#37458
Open
def- wants to merge 3 commits into
Open
Conversation
Follow-up to MaterializeInc#37440 Sorry for only realizing so late that it's an issue after all! `parse_oid` was widened to the full `u32` range (MaterializeInc#37440), but the same helper backs `CastStringToOid` in the PostgreSQL source, whose eval behavior must stay stable across releases. Replication re-casts the old tuple on delete, so a value ingested pre-upgrade as a `CastError` would be retracted post-upgrade as a value. That leaves the error stuck and adds a phantom negative row. Keep the storage cast on a frozen `parse_oid_legacy` that accepts only the `i32` range. The SQL, COPY, and pgwire paths keep the widened `parse_oid`. Add a unit snapshot pinning the frozen behavior and a platform-check reproducing the cross-upgrade retraction. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Freezing CastStringToOid on the legacy i32 range fixed retraction symmetry for existing sources, but also made fresh PostgreSQL sources reject valid oid values above i32::MAX, undoing the source-side part of the widening from MaterializeInc#37440 for all future sources. Version the cast per export instead. Newly purified exports persist cast_oid_full_range in their statement DETAILS and plan the new CastStringToOidFullRange, which accepts the full u32 range via the widened parse_oid. Exports whose details predate the flag decode as false and keep the legacy i32-range cast, so their persisted rows keep retracting cleanly. The cast choice derives from the persisted details rather than the binary version, which keeps replanning stable across restarts and upgrades. Add unit coverage for the full-range variant (error snapshot and parity with the SQL cast) and a pg-cdc testdrive file exercising snapshot, replication, and retraction of above-i32::MAX oids in scalar and array columns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The per-row cast error that the above-i32::MAX row produces on binaries with the legacy i32-range oid cast makes mz_source_statuses report the table export itself as stalled, while the source stays running and replication continues. The old assertion raced the running-to-stalled transition: sequential scenarios usually queried before the stall landed, but the parallel-mode shards and the earliest-release upgrade scenario saw stalled and failed in Nightly. Accept both states for the table export. Binaries whose exports carry cast_oid_full_range ingest the row and report running, legacy-cast binaries report stalled. The check's real assertion stays the validate SELECT that proves the delete cleanly retracts the error. Co-Authored-By: Claude Fable 5 <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
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.
Follow-up to #37440 Sorry for only realizing so late that it's an issue after all! Without this PR
bin/mzcompose --find platform-checks run default --scenario=UpgradeEntireMz --check=PgCdcOidRetractionfails like this after the upgrade:Since the source is not an errored state it's possible customers could run into this.
parse_oidwas widened to the fullu32range (#37440), but the same helper backsCastStringToOidin the PostgreSQL source, whose eval behavior must stay stable across releases. Replication re-casts the old tuple on delete, so a value ingested pre-upgrade as aCastErrorwould be retracted post-upgrade as a value. That leaves the error stuck and adds a phantom negative row.The fix versions the cast per export:
CastStringToOidis kept on a frozenparse_oid_legacythat accepts only thei32range, so exports created before the widening keep retracting cleanly across upgrades. The SQL, COPY, and pgwire paths keep the widenedparse_oid.cast_oid_full_rangeflag in their statementDETAILSand plan a newCastStringToOidFullRangevariant that accepts the fullu32range, so fresh sources ingest valid oid values abovei32::MAX. Column casts are regenerated from the persisted details on every plan, so deriving the choice from the details (rather than the binary version) keeps it stable for the lifetime of an export.Tests: a unit snapshot pinning the frozen legacy behavior, error snapshot and SQL-cast parity tests for the full-range variant, a platform-check reproducing the cross-upgrade retraction, and a pg-cdc testdrive file covering snapshot, replication, and retraction of above-
i32::MAXoids in scalar and array columns.