test: add a builtin-relation-diff validation harness - #38922
Draft
SangJunBak wants to merge 7 commits into
Draft
SangJunBak wants to merge 7 commits into
SangJunBak wants to merge 7 commits into
Conversation
Table-to-materialized-view conversions of builtin relations have to produce byte-identical contents, and nothing checked that. The harness dumps the configured relations from two fresh environments, one on a baseline image and one on the locally built code, applies the same corpus of user objects to both, and diffs the results. System ids are not stable across builds, so dumps are canonicalized before diffing: any cell that looks like a catalog id is rewritten to the qualified name of the object it denotes on that side, resolved through a per-namespace mapping. Rows that name an object the other side does not have at all are tolerated; everything else is reported. The relation set covers every conversion recorded in the MIGRATIONS list of builtin_schema_migration.rs. Relations the corpus cannot populate without external systems are empty on both sides, and their entries only validate the dump machinery against the relation's schema. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The hand-written corpus is a fixed set of statements and only grows by hand. `--workload` replaces it with a captured production workload replayed through the workload-replay framework, which reaches object shapes the hand-written corpus does not. Only the object-creation phase of the replay runs. The relations diffed here are catalog metadata, so the objects are the corpus and their contents are irrelevant. The seed is pinned rather than defaulted to the clock: a diff between two builds is meaningless if the corpus differs between them. Replay drives a single service named `materialized`, so the two sides run sequentially rather than side by side as in corpus mode, and the composition grows the services a capture's connections may reference. Those service definitions mirror test/workload-replay/mzcompose.py rather than using defaults: the framework creates Kafka topics from the host, so the broker needs a published host port and a HOST advertised listener, and Testdrive needs the vars the capture's generated DDL references. Replayed Postgres sources put a per-source UUID in their replication slot name, which reaches the dump hex-encoded inside the protobuf DETAILS option of create_sql, so it is scrubbed alongside the column itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add the relation now that its table-to-materialized-view conversion is in place. `builtin_rows_drift` applies because every builtin object holds a global id, so the builtin rows necessarily differ between two versions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Purification already replaces mz_now() in a REFRESH AT time or a REFRESH EVERY ... ALIGNED TO alignment with the literal the statement was planned at, and leaves any other expression as written. Fold the whole expression instead, storing <millis>::mz_timestamp. The stored create_sql is the only durable record of a refresh schedule, and the catalog views over mz_catalog_raw read it back without a planner, so the times have to be literals there. Planning a literal yields the same time, so the fold is idempotent, and an expression that does not fold is left for planning to reject with its usual error. Factor the planning of a refresh time into plan_refresh_time so the purifier and the planner share it. A fold can drop any reference the expression made (the target type of a cast, a function) besides adding mz_timestamp, so the purifier re-resolves the folded statement to recompute resolved_ids, which is what loading it from the catalog does. Add ast_rewrite_fold_refresh_times, a catalog migration that folds the times of materialized views stored before this change. Only name resolution needs the catalog: the expressions reference functions and types, never relations. SHOW CREATE MATERIALIZED VIEW now shows the folded time for a REFRESH AT built from an expression around mz_now(), which materialized_views.slt pins. The RefreshVariants platform check asserts that the schedule of a view created with literal times reads back exactly across upgrades. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ate_sql parse_catalog_create_sql gains the three fields the item-metadata catalog views need. retain_history_millis is the planned compaction window of a table, table from source, source, subsource, index or materialized view, mirroring plan_retain_history_option, with a disabled window reported as u64::MAX. replacement_target is the target of a REPLACEMENT FOR materialized view. refresh holds one entry per REFRESH option, with the EVERY interval rendered through the same duration round trip as planning and the times read from their mz_timestamp literals; a time still stored as an expression reads as NULL rather than failing every catalog view, which a 0dt read-only environment needs while it sees the leader's unmigrated catalog. Move the interval literal conversion from mz_sql::plan::literal into Interval::from_literal in mz_repr, which mz_expr can reach. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
mz_history_retention_strategies, mz_materialized_view_refresh_strategies and mz_replacements become materialized views over mz_catalog_raw, reading the RETAIN HISTORY, REFRESH and REPLACEMENT FOR clauses that parse_catalog_create_sql reports. Their packers go away and each gets a replacement migration step. Builtins have no create_sql. A builtin table, source or index flagged as a retained-metrics object follows the metrics_retention system parameter, read from mz_overridden_system_parameters with the compiled-in default inlined, and every other builtin has the default window. To make the flag visible to SQL, mz_builtin_tables and mz_builtin_sources gain an is_retained_metrics_object column and a generated mz_builtin_indexes view joins them. Builtin materialized views stay out of the retained set: CatalogItem::is_retained_metrics_object is false for materialized views, so the flag is inert for them and the table always reported the default window, which the view keeps. An override of metrics_retention set through --system-parameter-default is not durable and so invisible here; the view's comment records that. Closes: SQL-499 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Add sqllogictests for mz_replacements, mz_history_retention_strategies and mz_materialized_view_refresh_strategies: the schema, one row per RETAIN HISTORY spelling and per REFRESH option, builtin and introspection rows, a metrics_retention override, the folded create_sql, and retraction. Extend the builtin-relation-diff corpus with the same cases and register the three relations, plus mz_source_references with its wall-clock updated_at ignored. The harness also learns what the diff against the merge base needed: to drop the id inside a `[<id> AS <name>]` reference in create_sql, which shifts whenever a builtin is added ahead of the object; to scrub the per-environment SSH keys an SSH tunnel connection's create_sql spells out; to tolerate a one-sided row about a builtin that is an object of another kind on the other side, which is what a builtin table that became a materialized view looks like; to name the --user-rows-only filter column per relation; and to accept the refresh-time fold in mz_materialized_views.create_sql. Co-Authored-By: Claude Fable 5.1 <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.
Derive the CatalogItemId -> GlobalId mapping from
mz_catalog_rawrather than packing it from the in-memory catalog. Four durable
sources make up the mapping:
Itemrows, whoseglobal_idis the root version's GlobalId. Thesecarry user items, temporary items, and the runtime-alterable
builtins, which are durably recorded in the items collection.
extra_versionson those rows, one GlobalId per later version of atable or materialized view. This is the only case where one
CatalogItemId maps to several GlobalIds.
GidMappingrows for the remaining builtins. Runtime-alterable onesare excluded by their sentinel fingerprint: their
Itemrow alreadycarries the same pair, so keeping both would double-count.
ClusterIntrospectionSourceIndexrows.UNION ALL combines the branches. A GlobalId is allocated once and
belongs to exactly one item, so the branches are disjoint. The desc
declares no key even though
global_idis unique, because a UNION ALLover jsonb extractions does not prove one and
verify_builtin_descscompares the declared desc against what the optimizer infers. The new
sqllogictest file asserts the uniqueness invariant instead.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.comRemove these sections if your commit already has a good description!
Motivation
Why does this change exist? Link to a GitHub issue, design doc, Slack
thread, or explain the problem in a sentence or two. A reviewer who has
no context should understand why after reading this section.
If this implements or addresses an existing issue, it's enough to link to that:
Closes
Fixes
etc.
Description
What does this PR actually do? Focus on the approach and any non-obvious
decisions. The diff shows the code --- use this space to explain what the
diff can't tell a reviewer.
Verification
How do you know this change is correct? Describe new or existing automated
tests, or manual steps you took.
Table-to-materialized-view conversions of builtin relations have to produce
byte-identical contents, and nothing checked that. The harness dumps the
configured relations from two fresh environments, one on a baseline image and
one on the locally built code, applies the same corpus of user objects to
both, and diffs the results.
System ids are not stable across builds, so dumps are canonicalized before
diffing: any cell that looks like a catalog id is rewritten to the qualified
name of the object it denotes on that side, resolved through a per-namespace
mapping. Rows that name an object the other side does not have at all are
tolerated; everything else is reported.
The relation set covers every conversion recorded in the MIGRATIONS list of
builtin_schema_migration.rs. Relations the corpus cannot populate without
external systems are empty on both sides, and their entries only validate the
dump machinery against the relation's schema.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com