Diff builtin migrations using workload replay - #38489
Draft
SangJunBak wants to merge 7 commits into
Draft
Conversation
**Context:** When resolving a catalog item's statement, anytime we recorded a reference to an array, we'd record both its array type (e.g. _int4) and the element type (int4). We did this to fix a bug ( MaterializeInc#25739) where we'd save a statement `CREATE TABLE t (a int4[])` as `CREATE ... _int4 ` durably. This means in memory, we'd resolve int4[] to int4 and from the durable catalog, _int4 to _int4, resulting in a catalog inconsistency between memory and durable. The fix was to resolve int4[] to _int4 and int4, and _int4 to both as well. However, my change is to have both resolve to _int4. **Repercussions of my solution:** Resolvers ared used in user-visible ways via: - Drop protection: Ensures we can't drop anything a catalog item references - RBAC: Enforces RBAC rules based on references - Observability: We show a relationship in mz_object_dependencies from the catalog item to _int4 and int4 - Consistency between in-memory and the durable catalog (for the bug fix) However we can never drop types (1), RBAC is still enforced since we default usage of all array types to public (2), it doesn't make sense to show an edge to the underlying type (3), and we maintain consistency since the mappings are symmetrical (4). **Why I'm doing this change:** We would've had inconsistent logic in what edges to show in mz_object_dependencies between builtin objects and user objects since user objects get stored as _int4 while builtin objects resolves from int4[] since it's statically defined. Having both resolve to _int4 fixes this inconsistency. **What my change does** - When recursively resolving the catalog item, continue the recursion but short circuit the recording by passing "false" as soon as we recurse into the array type. - Create a test to make round tripping between memory and the durable catalog produces the same resolved IDs
Walks a raw statement AST and buckets every catalog item reference by how it is named: bracketed id references, and name-only references in relation, function, and type position. CTE bindings are excluded from relation references using the same lexical scoping rules as name resolution. Groundwork for SQL-150: deriving mz_object_dependencies from stored create_sql in a builtin materialized view over mz_catalog_raw.
Extracts catalog item references from a stored create_sql string as a JSONB object with ids, funcs, types, and relations fields. Function references print as plain qualified names in stored SQL, so downstream consumers recover them by joining name against GidMapping rows. The types and relations fields are defensive, stored SQL prints both as id references. The golden churn in mz_catalog_server_index_accounting.slt is the fresh-install system id shift from adding one builtin function. Part of SQL-150.
Part of SQL-150 (multi-envd): replace the coordinator-packed
mz_object_dependencies builtin table with a builtin materialized view
over mz_internal.mz_catalog_raw.
The edges live in a new builtin view, mz_internal.mz_object_dependencies_raw,
which unions four edge sources: user item references extracted from stored
create_sql via parse_catalog_item_references, builtin edges collected at
static init by parsing every SQL-bearing builtin and inlined as VALUES,
introspection source index edges, and function references recovered by name
against GidMapping rows. mz_object_dependencies is a bare read of that view.
Splitting it that way keeps the materialized view's definition, and so its
catalog fingerprint, fixed across releases even though the inlined builtin
edges change whenever a builtin's references do. The materialized view keeps
its persist shard through an upgrade and the self-correcting persist sink
writes the difference once the edge set changes, so no new
MigrationStep::replacement is needed each time a builtin view is edited. The
view carries no durable state of its own and is rebuilt from its definition
on every boot.
Semantic changes vs the packed table: array element type edges of user
items are dropped (the element id is injected at resolution and never
printed), and mz_object_dependencies_raw lists no outgoing edges for itself
(the table had none either).
Manual verification of the cross-version contents, since no automated test
covers a builtin reference change across a restart:
1. bin/environmentd --reset, then record the baseline through psql on 6875:
SELECT ro.name
FROM mz_internal.mz_object_dependencies d
JOIN mz_catalog.mz_objects o ON o.id = d.object_id
JOIN mz_catalog.mz_objects ro ON ro.id = d.referenced_object_id
WHERE o.name = 'mz_object_lifetimes';
SELECT count(*) FROM mz_internal.mz_object_dependencies
WHERE object_id LIKE 's%';
SELECT s.shard_id FROM mz_internal.mz_object_global_ids g
JOIN mz_internal.mz_storage_shards s ON s.object_id = g.global_id
WHERE g.id = (SELECT id FROM mz_catalog.mz_objects
WHERE name = 'mz_object_dependencies'
AND type = 'materialized-view');
Baseline: one edge to mz_audit_events, 1210 builtin edges, shard
s6b318b63-fd45-4944-ade5-6a2966607c3e.
2. Stand in for the next release by giving a builtin view a reference it did
not have. MZ_OBJECT_LIFETIMES gains a no-op predicate:
AND NOT EXISTS (SELECT 1 FROM mz_catalog.mz_databases WHERE false)
The reference target must appear earlier in BUILTINS_STATIC than the view
that gains it, or bootstrap planning fails. Rebuild, then restart WITHOUT
--reset so the catalog and the persist shard survive.
Observed: boot clean, no fingerprint mismatch and no migration step run;
a new edge to mz_databases; 1211 builtin edges; the same shard id, so the
sink corrected the existing shard rather than a replacement being made.
3. Revert the predicate, rebuild, restart without --reset. This is the half
that matters, since appending rows proves less than retracting them.
Observed: the mz_databases edge is gone, back to 1210 builtin edges, still
the same shard, boot clean.
The durable fingerprint reflects the split directly. Read from a GidMapping
row in mz_catalog_raw as mz_system, mz_object_dependencies now carries a
256 byte fingerprint, against 14732 for mz_indexes and 13492 for mz_sources,
which still inline their VALUES and so still move on every builtin addition.
Generic mzcompose composition for validating builtin-table-to- materialized-view migrations: dump configured relations from a fresh baseline environment and a fresh locally built environment after applying the same user object corpus, canonicalize system ids to qualified names, and diff. Per-relation allowlists explain expected differences, starting with the dropped array element type edges of mz_object_dependencies. design doc: mz_object_dependencies conversion (local only, do not push)
The hand-written CORPUS covers the edge classes the mz_object_dependencies conversion had to reproduce deliberately, but it is small: a dozen statements against no external systems. A captured workload is a far richer corpus of user objects, which is what exercises the create_sql-parsing path the conversion relies on. --workload swaps the corpus for a replayed capture. It stays opt-in because the replay needs the captured-workloads repo, brings up Kafka, Postgres, MySQL and an SSH bastion, and cannot cover temporary items: those are session-scoped and never appear in a catalog capture, so the CORPUS remains the only coverage of the ephemeral_owner_session filter.
The workload mode needs the same Materialized console port and the same host-reachable Kafka broker the replay composition declares: the framework prints the console URL via c.port(6874), and creates topics from the host with confluent_kafka.admin, so PLAINTEXT://kafka:9092 alone is unreachable. Testdrive's vars and SqlServer come along for the same reason.
SangJunBak
added a commit
that referenced
this pull request
Sep 3, 2026
I've tried my best to explain each change in the commit messages, but I've also shared some nicely formatted explanations that I used to understand the changes myself: - Why we don't resolve an array type to both its array type (e.g. _int4) and its element type (int4) anymore (commit 1): https://claude.ai/code/artifact/f9d39cf2-eff9-4a80-b5e0-c712183a24bd - How we were able to recreate the dependency graph via a SQL function (commits >= 2): https://claude.ai/code/artifact/f3b59447-9924-49d6-8085-1db9dff69048 . Ignore the diff harness comment. You might notice some code is different, but the motivation/high level approach is the same. ### Motivation Partially does sql-499 ### Verification - More SLTs for testing the ported builtin more in depth - Unit tests for the parser changes - I created a harness that diffs builtins for all system objects, and user objects created from Dennis' workload replay. It's in draft #38489 but used it and things were 1:1
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.
Context:
When resolving a catalog item's statement, anytime we recorded a reference to an array, we'd record both its array type (e.g. _int4) and the element type (int4). We did this to fix a bug ( #25739) where we'd save a statement
CREATE TABLE t (a int4[])asCREATE ... _int4durably. This means in memory, we'd resolve int4[] to int4 and from the durable catalog, _int4 to _int4, resulting in a catalog inconsistency between memory and durable. The fix was to resolve int4[] to _int4 and int4, and _int4 to both as well. However, my change is to have both resolve to _int4.Repercussions of my solution:
Resolvers ared used in user-visible ways via:
However we can never drop types (1), RBAC is still enforced since we default usage of all array types to public (2), it doesn't make sense to show an edge to the underlying type (3), and we maintain consistency since the mappings are symmetrical (4).
Why I'm doing this change:
We would've had inconsistent logic in what edges to show in mz_object_dependencies between builtin objects and user objects since user objects get stored as _int4 while builtin objects resolves from int4[] since it's statically defined. Having both resolve to _int4 fixes this inconsistency.
What my change does
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.