sql-499: Convert mz_object_dependencies by SangJunBak · Pull Request #38252 · MaterializeInc/materialize

SangJunBak · 2026-08-17T18:13:53Z

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 Diff builtin migrations using workload replay #38489 but used it and things were 1:1

linear-code · 2026-08-17T18:14:13Z

SQL-150

SQL-499

mtabebe

Some comments/clarifications from me.

This does look good to me. I found the artifacts helpful in walking through and the decomposition makes sense.

@ggevay's agent always finds interesting things... not sure what he prompts to get it. So might be worth him poking at it.

The json parsing and SQL is pretty dense (my agent was satisfied with it from a correctness POV) and there are lots of tests. So hoping nothing got lost in the sauce.

mtabebe · 2026-08-28T16:30:16Z

+            // so this is what keeps the reloaded catalog identical to the
+            // in-memory one.
+            assert_eq!(
+                ids_by_spelling[ARRAY_SUFFIX], ids_by_spelling[ARRAY_TYPE],


Awesome that we are enforcing this in test

mtabebe · 2026-08-28T16:46:25Z

+    /// Catalog IDs from `RawItemName::Id` references
+    pub ids: BTreeSet<String>,
+    /// Named relations that don't carry an ID. Most commonly seen in builtin statements since they
+    /// are not normalized. CTE names are excluded.


It would be helpful to explain why CTE names are excluded your artifact says this:

The subtle part is CTE handling: a name bound by a WITH is not a catalog reference at all, so it must be excluded — and excluded under exactly the resolver's own scoping rules. A plain WITH binds each name only after its own definition, WITH MUTUALLY RECURSIVE binds all names up front, and the bindings vanish at the end of the query. The collector keeps a stack and truncates it on the way out, so a sibling scope's identically-named relation still counts.

Addressed in sql-parser: explain why CTE names are excluded from item references: the field doc now says a WITH-bound name refers to the query-local binding rather than a catalog item (so recording it would fabricate an edge to any same-named catalog object), and the comment at visit_query spells out the scoping rules the exclusion mirrors: plain WITH binds each name only after its own definition, WITH MUTUALLY RECURSIVE binds all names up front, and both bindings end with the query, so a sibling scope's identically-named relation is still recorded.

mtabebe · 2026-08-28T16:54:04Z

+
+    fn visit_function(&mut self, node: &'ast Function<Raw>) {
+        self.record(&node.name, Position::Func);
+        // Visit everything `visit::visit_function` visits


JFYI my claude is very concerned that in the future there might be drift with this.

I don't think we need to do anything about it, but if there is a cheap way to enforce this, it might be nice for avoiding future bugs.

Addressed in sql-parser: make the reference collector's hand-rolled visits drift-proof: visit_function and visit_query now destructure their nodes exhaustively instead of reaching through the reference, so a field added to Function or Query fails compilation at the collector instead of being silently skipped by the hand-rolled traversal.

mtabebe · 2026-08-28T16:54:35Z


+/// Extracts the catalog item references from a catalog `create_sql` string.
+///
+/// Returns a JSONB object whose fields mirror `ItemReferences`.


nit but named_array_elements is dropped (which makes sense)

It is sound today because both ResolvedDataType::Named construction sites print with an id and there is no Array variant, so persisted SQL never carries T[]; but the two sides now disagree silently. Either emit the bucket too or soft_assert_or_log that it is empty, with that invariant stated next to it.

Went with the assert, in expr: assert the array-element bucket is empty in parse_catalog_item_references: the doc comment now states the invariant (stored catalog SQL never spells an array type as T[], since name resolution prints a resolved array type as a single id reference) and the body soft-asserts the bucket is empty, so a resolver change that starts persisting that spelling logs instead of silently dropping the reference.

mtabebe · 2026-08-28T17:09:24Z

+        CROSS JOIN LATERAL jsonb_array_elements(u.refs->'named_funcs') AS f(func)
+        JOIN gid_mappings gm ON
+            gm.object_type = '{func_type}' AND
+            gm.schema_name = f.func->>'schema' AND


This is safe because we always store function names as enforced by test/sqllogictest/funcs.slt?

By construction rather than by test: resolve_item_name_name sets print_id = !(Func | Type) and a function's full_name is always ambient database + schema + item, so a function reference persists as exactly "schema"."name" (and normalize_function_name qualified them even before the id syntax existed). The one caveat is in the thread on item_refs.rs:89: types resolved in DOC ON positions print the same way, and those do not get an edge today.

Recorded the by-construction argument in catalog: document why joining function references by name is sound: the generator doc now states that name resolution never prints an id for a function (print_id is false for Func) and a function's full_name always carries its schema, so a function reference persists as exactly "schema"."name" rather than relying on the SLT for enforcement.

mtabebe · 2026-08-28T17:11:22Z

+            gm.data->'key'->>'object_name' = isi.data->'key'->>'name'
+        WHERE isi.data->>'kind' = 'ClusterIntrospectionSourceIndex'
+    )
+SELECT object_id, referenced_object_id FROM user_id_edges


UNION ALL is safe because these are all disjoint, so that makes sense. Might just be worth a comment.

Disjointness holds only while every name-based position is disjoint from the id-based ones, and there is one spelling where that is not guaranteed: a table function written by id persists that way (parse_table_factor_inner uses parse_raw_name, and the id path prints print_id: true), so [sNN AS pg_catalog.generate_series](..) next to the same function by name in one statement lands the edge in both ids and named_funcs, and UNION ALL emits it twice, breaking the no-duplicates check the SLT asserts. Contrived, but UNION here costs a small distinct and removes the assumption; otherwise the comment Michael suggests, stating the assumption.

Switched to UNION in catalog: use UNION instead of UNION ALL in mz_object_dependencies_raw, with a comment at the union stating why. The same commit adds a regression test to test/testdrive/mz-depends.td for the one overlapping spelling: a view over [<id> AS pg_catalog.generate_series](...) next to generate_series(...) by name. It asserts both spellings persist into the stored create_sql (the id in ids, the name in named_funcs) and that the view still records a single edge. Verified red by running the view definition with UNION swapped back to UNION ALL as an ad-hoc query against the same catalog, which returns that edge twice.

mtabebe · 2026-08-28T17:13:55Z

+query T
+SELECT count(*) FROM mz_internal.mz_object_dependencies WHERE object_id = referenced_object_id
+----
+0


Are there any tests in here for the correct behaviour after a drop of a view?

+1. A few more that are all creatable in SLT and each hit a different path: DROP VIEW v then expect 0 edges; a user materialized view; an index with a function in its key (CREATE INDEX ON t (abs(a)), function edge on an Index item); CREATE TYPE l AS LIST (ELEMENT TYPE = int4) (type -> type edge); and CREATE SOURCE lg FROM LOAD GENERATOR COUNTER plus CREATE TABLE lg_tbl FROM SOURCE lg (REFERENCE counter) (id reference outside a query; mz_tables.slt already does this). mz-depends.td covers sources, subsources, and connections with real upstreams.

I can assert the drop state for the tests:

Temporary objects (asserting there's still no dependency edge)

connection and secret edges

Create a respect

And I can cover the each of the tests Gabor mentioned, each in their respective files or at the very end of mz_object_dependencies.slt

Added in test: extend mz_object_dependencies SLT coverage: a user materialized view, an index whose key calls a function (function edge on an Index item), a custom LIST type (type to type edge), a table created from a source (id reference outside a query), and drop coverage. The file now creates everything in a scratch schema and ends with a DROP SCHEMA CASCADE followed by an assertion that no user edge remains, so a targeted drop is checked mid-file and any object a future test adds is automatically covered by the final leak check. Temporary objects and connection/secret edges were already covered earlier in the file.

mtabebe · 2026-08-28T17:15:10Z

+        collector.collect(b.schema(), b.name(), object_type, &create_sql);
+    }
+
+    // We include `mz_object_dependencies` own edges into `mz_object_dependencies`.


This read weirdly to me and I wasn't sure if you meant mz_object_dependencies_raw? (given the function we are in)

Oops I meant mz_object_dependencies_raw!

Fixed in catalog: fix which view the self-edges comment names: the comment now says the second pass appends mz_object_dependencies_raw's own outgoing edges to its VALUES rows, since the view is generated there and the builtin walk never saw it.

ggevay

Wrote some comments, mostly Claude

ggevay · 2026-08-31T10:52:14Z

+        // NOTE above: this version must stay at the workspace's current dev
+        // version until the change ships.
+        MigrationStep::replacement(
+            "26.40.0-dev.0",


Main is at 26.41.0-dev.0 since #38567 (Thursday 08-27), so this needs to be "26.41.0-dev.0" when you rebase. plan_migration keeps only steps with version > source_version, and 26.40.0-dev.0 < 26.40.0, so on the real 26.40.x -> 26.41 upgrade this step is dropped, and update_fingerprints then panics on the MV's changed fingerprint (not migrated, not ephemeral, not runtime-alterable): the new version fails to open the catalog. Same trap as #37465. The upgrade tests passed only because they ran before the bump; the 0dt smoke test will go red on the first CI run after the rebase if this stays.

Fixed in adapter: move the mz_object_dependencies migration step to 26.41.0-dev.0, after rebasing the stack onto latest main. Thanks for catching this before the 0dt smoke test did.

ggevay · 2026-08-31T10:52:14Z

 SELECT id, global_id FROM mz_internal.mz_dataflow_global_ids ORDER BY id, global_id;
 ----
-11  t<id>
+12  t<id>


Please revert this hunk, it is not caused by this PR and it is what makes nightly SLT (2 replicas) 7 red here (build 18173 got 11 against this 12). This file only runs in nightly (the fast config excludes it as too slow), and main's own nightlies flip on exactly this line: 18169 passed with 11, 18175 and 18176 failed with "expected 11, actually 12", 18177 passed with 11 again. So it is a main-side flake since ~08-28, best handled separately.

Reverted in test: revert unrelated dataflow-count bump in attribution SLT.

ggevay · 2026-08-31T10:52:14Z

+
+impl<'ast> Visit<'ast, Raw> for ReferenceCollector {
+    fn visit_item_name(&mut self, node: &'ast RawItemName) {
+        self.record(node, Position::Relation);


This is the one place where the parity story breaks. fold_doc_on_identifier (names.rs) resolves DOC ON TYPE x with types: true, relations: true, and fold_column_name does the same for DOC ON COLUMN x.c; resolve_item_name_name prints Types without an id (print_id = !(Func | Type)), so a type in those positions persists as a bare "materialize"."public"."point" while a relation persists as [u1 AS ...]. The resolver still inserts the type's id, so the old table had sink -> point. Here the bare name is filed under named_relations, and user_relation_edges only joins GidMapping rows with object_type NOT IN (Type, Func), so a user type (no GidMapping row at all) and a builtin type both drop out silently.

Purification injects DOC ON TYPE/COLUMN for every commented item in from.references(), column types included (purify_create_sink_avro_doc_on_options), so any Avro/CSR sink over a relation whose user-defined column type carries a COMMENT hits this without the user writing DOC ON. Persisted shape: test/testdrive/kafka-avro-sinks-doc-comments.td:57. The edge is a real dependency (uses() for a sink is references(), and it is what blocks DROP TYPE point), so it should stay in the view.

Two fix shapes: (a) give DocOnIdentifier::Type and ColumnName.relation their own position and, for user schemas, resolve the name against Item rows in mz_catalog_raw (name plus schema via the Schema rows), allowing Type rows in that position; (b) print ids in the DOC ON positions from now on plus an AST migration for existing sinks. (a) stays inside the view, (b) is cleaner long term. Either way an assertion on mz_object_dependencies in kafka-avro-sinks-doc-comments.td would pin it.

The #38489 harness corpus has COMMENT ON TABLE/COMMENT ON COLUMN and a CREATE TYPE, but no commented user type and no Avro/CSR sink over a relation that uses one, which is why it reported 1:1 here; adding COMMENT ON TYPE int4_list ... plus such a sink to the corpus would make it catch this class.

Could we perhaps do b) in a followup PR right after? Just because the consequences of not reporting DOC ON types/columns seems non-blocking

Created a followup PR #38612

ggevay · 2026-08-31T10:52:14Z

+    let self_rows = self_edges(&mz_object_dependencies_raw_sql(&collector.rows));
+    let mut rows = collector.rows;
+    rows.extend(self_rows.iter().cloned());
+    let sql = mz_object_dependencies_raw_sql(&rows);


The commit message says "an assert in the generator holds" the second-pass convergence, but nothing here checks it. Since the appended rows are VALUES literals it does converge, and asserting it is one more parse of this view's SQL at init, which self_edges already does once: assert_eq!(self_edges(&sql), self_rows, "..."). Either that or drop the sentence; test_mz_object_dependencies_raw_sql_is_stable covers preview-vs-real inputs, not this.

I'll drop the sentence!

Dropped the sentence from the commit message of catalog, adapter: convert mz_object_dependencies to a materialized view; the convergence claim now rests only on the appended rows being VALUES literals that reference nothing themselves.

ggevay · 2026-08-31T10:52:14Z

+    serde_json::to_string(&object_type).expect("CatalogItemType is serializable")
+}
+
+/// Row of a builtin


Nit: this reads as if it were cut off. Something like "One builtin dependency edge, inlined as a VALUES row of mz_object_dependencies_raw" says what the row is.

Fixed in catalog: finish the BuiltinEdgeRow doc comment, using your wording.

ggevay · 2026-08-31T10:52:14Z

+            # a non-owner tries this drop) will raise an error and
+            # we'll retry the DropIndexAction.
+            exe.execute(f"DROP INDEX IF EXISTS {index}", http=Http.RANDOM)
+            #  Use discard, not remove: a concurrent


Nit: double space after #, and the commit message sentence "We use IF EXISTS since so those succeed if an entry still does exist and the stale entry is untracked" reads garbled. The change itself is right: resolve_item_or_type returns Ok(None) on any resolution error under IF EXISTS, including a vanished schema, so untracking after success is safe and RBAC errors still keep the entry.

Both fixed: the comment whitespace in parallel-workload: fix comment indentation nit, and the garbled sentence in the commit message of parallel-workload: keep DropIndexAction from forgetting live indexes now reads: the drop uses IF EXISTS, so it succeeds whether the index still exists or already vanished concurrently, the entry is untracked in both cases, and an RBAC failure still raises and keeps the index tracked for a later retry.

ggevay · 2026-08-31T10:52:27Z

+            gm.data->'key'->>'object_name' = isi.data->'key'->>'name'
+        WHERE isi.data->>'kind' = 'ClusterIntrospectionSourceIndex'
+    )
+SELECT object_id, referenced_object_id FROM user_id_edges


Disjointness holds only while every name-based position is disjoint from the id-based ones, and there is one spelling where that is not guaranteed: a table function written by id persists that way (parse_table_factor_inner uses parse_raw_name, and the id path prints print_id: true), so [sNN AS pg_catalog.generate_series](..) next to the same function by name in one statement lands the edge in both ids and named_funcs, and UNION ALL emits it twice, breaking the no-duplicates check the SLT asserts. Contrived, but UNION here costs a small distinct and removes the assumption; otherwise the comment Michael suggests, stating the assumption.

ggevay · 2026-08-31T10:52:30Z


+/// Extracts the catalog item references from a catalog `create_sql` string.
+///
+/// Returns a JSONB object whose fields mirror `ItemReferences`.


It is sound today because both ResolvedDataType::Named construction sites print with an id and there is no Array variant, so persisted SQL never carries T[]; but the two sides now disagree silently. Either emit the bucket too or soft_assert_or_log that it is empty, with that invariant stated next to it.

ggevay · 2026-08-31T10:52:33Z

+query T
+SELECT count(*) FROM mz_internal.mz_object_dependencies WHERE object_id = referenced_object_id
+----
+0


+1. A few more that are all creatable in SLT and each hit a different path: DROP VIEW v then expect 0 edges; a user materialized view; an index with a function in its key (CREATE INDEX ON t (abs(a)), function edge on an Index item); CREATE TYPE l AS LIST (ELEMENT TYPE = int4) (type -> type edge); and CREATE SOURCE lg FROM LOAD GENERATOR COUNTER plus CREATE TABLE lg_tbl FROM SOURCE lg (REFERENCE counter) (id reference outside a query; mz_tables.slt already does this). mz-depends.td covers sources, subsources, and connections with real upstreams.

ggevay · 2026-08-31T10:52:36Z

+        CROSS JOIN LATERAL jsonb_array_elements(u.refs->'named_funcs') AS f(func)
+        JOIN gid_mappings gm ON
+            gm.object_type = '{func_type}' AND
+            gm.schema_name = f.func->>'schema' AND


By construction rather than by test: resolve_item_name_name sets print_id = !(Func | Type) and a function's full_name is always ambient database + schema + item, so a function reference persists as exactly "schema"."name" (and normalize_function_name qualified them even before the id syntax existed). The one caveat is in the thread on item_refs.rs:89: types resolved in DOC ON positions print the same way, and those do not get an edge today.

ggevay · 2026-08-31T12:49:53Z

@ggevay's agent always finds interesting things... not sure what he prompts to get it.

I usually ask it to review "carefully", and I ask about confidence numbers for each draft comment and the overall verdict, and then I often push it to do more reading/thinking/experiments if the confidences are below 95-97%. And I always use the same session for these builtin conversion PRs, so it remembers earlier things (I ask it to record things carefully when doing a compaction, often in several rounds), plus I also used this session for earlier builtin conversion incidents.

def- · 2026-09-01T18:20:57Z

QA LLM Review

1. MEDIUM -- soft assert on user input in `mz_internal.parse_catalog_item_references`

src/expr/src/scalar/func/impls/jsonb.rs:844

The new soft_assert_or_log! asserts an invariant of persisted catalog create_sql (that named_array_elements is always empty) inside a scalar function any user can call on arbitrary text. SELECT mz_internal.parse_catalog_item_references('CREATE TABLE t (a int[])') violates it, so in any build with soft assertions enabled it becomes an assert! and aborts the process evaluating it, and in release builds it becomes an unbounded, user-driven stream of tracing::error! events that the Sentry tracing layer picks up.

Details

mz_internal has USAGE for PUBLIC and the function is registered in MZ_INTERNAL_BUILTINS (src/sql/src/func.rs:5358), so no privilege is needed; the PR's own test/sqllogictest/funcs.slt calls it directly. named_array_elements is populated for any T[] spelled with a plain name (src/sql-parser/src/ast/item_refs.rs:123), which arbitrary input trivially produces.

Where the abort lands depends on how the argument is evaluated. A literal argument is constant-folded in environmentd, and mz_transform::catch_unwind_optimize demotes that panic to an internal error plus a "caught a panic during query optimization" error log. A non-literal argument is not: CREATE MATERIALIZED VIEW mv AS SELECT mz_internal.parse_catalog_item_references(s) FROM tbl evaluates it in a dataflow operator, and mz_ore::panic::install_enhanced_handler aborts the whole process on any thread panic, so the replica crash-loops across restarts until the MV is dropped from another cluster. Soft assertions are on by default in every mzcompose-launched Materialize (misc/python/materialize/mzcompose/services/materialized.py:110), in clusterd.py, sql_logic_test.py, cloudtest, and local bin/environmentd.

The invariant itself is right for the caller that motivated it: every ResolvedDataType::Named construction site sets print_id: true (src/sql/src/names.rs:1393 and :1453, src/sql/src/pure.rs:2857, src/sql/src/plan/transform_ast.rs:87), so stored SQL spells an array type as a single id reference and mz_object_dependencies_raw never trips it. The problem is only that the check sits at a public entry point rather than at that caller.

The simplest fix is to stop discarding the bucket instead of asserting about it, which also makes the JSONB a true mirror of ItemReferences again; the view just ignores the extra field:

-        mz_ore::soft_assert_or_log!(
-            refs.named_array_elements.is_empty(),
-            "persisted create_sql should never carry an array type T[] \
-            and resolve its array type in `ids`: {:?}",
-            refs.named_array_elements
-        );
         Ok(json!({
             "ids": refs.ids.iter().collect::<Vec<_>>(),
             "named_funcs": refs.named_funcs.iter().map(qualified).collect::<Vec<_>>(),
             "named_types": refs.named_types.iter().map(qualified).collect::<Vec<_>>(),
             "named_relations": refs.named_relations.iter().map(qualified).collect::<Vec<_>>(),
+            "named_array_elements": refs.named_array_elements.iter().map(qualified).collect::<Vec<_>>(),
         }))

If you want to keep the alarm, move it to the generator side (BuiltinEdgeCollector::collect / make_mz_object_dependencies_raw), where the input really is catalog-authored.

SangJunBak · 2026-09-01T18:56:52Z

I'm not sure how valid this review comment is. We don't expect users to call mz_internal.parse_catalog_item_references and even if they do, it's not necessarily broken since we expect a certain input for the function and don't document that we accept T[].

ggevay

LGTM, just two minor things left.

ggevay · 2026-09-02T11:07:06Z

+        name: MZ_OBJECT_DEPENDENCIES_RAW,
+        schema: MZ_INTERNAL_SCHEMA,
+        oid: oid::VIEW_MZ_OBJECT_DEPENDENCIES_RAW_OID,
+        desc: RelationDesc::builder()


CI: verify_builtin_descs fails since the UNION switch, the planned desc now carries the key [[0, 1]] (Distinct over both columns) while this one declares none. .with_key(vec![0, 1]) here, and I believe on the MV desc below as well since the key propagates through the bare projection; MZ_RECENT_SQL_TEXT is the precedent (SELECT DISTINCT with .with_key(vec![0, 1, 2])). Same root for the two catalog_server_explain.slt goldens (a Distinct GroupAggregate on top), which now also conflict with main, as does oid.rs (#38512 took 17124).

ggevay · 2026-09-02T11:07:06Z

+            gm.data->'key'->>'object_name' = isi.data->'key'->>'name'
+        WHERE isi.data->>'kind' = 'ClusterIntrospectionSourceIndex'
+    )
+-- UNION rather than UNION ALL: the branches are disjoint for the most part, but a


Nit: the FROM one has to be spelled by id for this to happen, [sNN AS pg_catalog.generate_series](...) as mz-depends.td does; by name it resolves through fold_function with print_id: false like any other call. The commit message has it the other way round.

**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[])` 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 that round tripping between memory and the durable catalog produces the same resolved IDs. It also asserts the two spellings agree with each other (int4[] and _int4 resolve to the same ids) and that neither records the element type, which is the invariant the deleted code existed to protect.

Walks a raw statement AST and buckets every catalog item reference. CTE bindings are excluded.

Extracts catalog item references from a stored create_sql string as a JSONB object. Named references (e.g. named_funcs) print as plain qualified names in stored SQL, so downstream consumers recover them by joining (schema, name) against GidMapping rows.

- Extracts `assert_safe_builtin_name` out of `mz_catalog.rs` to be shared with `mz_internal.rs` 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 three 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, and introspection source index edges. Function, type, and relation references print as plain qualified names rather than ids, so they are recovered by joining (schema, 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. The generator runs in two passes. mz_object_dependencies_raw is generated here, so it is absent from the builtin list the generator walks and its own edges can only be read off its finished SQL; a second pass appends them. That converges, because the appended rows are VALUES literals and reference nothing themselves. Without it a transitive walk from mz_object_dependencies would stop at the view instead of reaching mz_catalog_raw. The object_type codes the view compares GidMapping rows against are read off the ProtoCatalogItemType enum rather than spelled as literals, so a renumbering there cannot leave the view silently matching the wrong kinds. The relation-position match is written as an exclusion of the type and func codes, so a kind nobody anticipated (a secret or connection named by a builtin connection, say) still gets its edge rather than none. Semantic change vs the packed table: array element type edges of user items are dropped, since the element id is injected at resolution and never printed. 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.

DropIndexAction untracked an index on any DROP INDEX failure, on the assumption that the only way the statement fails is the index having vanished under a concurrent drop of its object or schema. That is not the only way. Workers spend long stretches connected as random roles, and a non-owner's DROP INDEX fails with "must be owner of INDEX". The action tolerated that error but still discarded the index, so a single non-owner attempt removed a perfectly good index from tracking for the rest of the run while it kept existing in the catalog. The drop now uses IF EXISTS, so it succeeds whether the index still exists or already vanished concurrently, and the entry is untracked in both cases. An RBAC failure still raises, keeping the index tracked for a later retry.

Review feedback: state why a name bound by WITH must not be recorded, and that the exclusion mirrors the name resolver's scoping rules.

…roof The `visit_function` and `visit_query` destructures its node exhaustively so that a field added to `Function` or `Query` fails compilation here instead of being silently skipped.

…references Review feedback: the function's JSONB output mirrors every ItemReferences field except named_array_elements, and the two sides disagreed silently. The bucket is empty for stored catalog SQL by construction, since both ResolvedDataType::Named construction sites print with an id and there is no Array variant, so no persisted statement spells an array type as T[]. State that invariant in the contract and soft-assert it, so a resolver change that starts persisting the spelling logs instead of silently dropping the reference.

A function reference prints its id only when the statement spells it that way, as `FROM [sNN AS pg_catalog.generate_series](...)`. A plain call to the same function resolves by name instead, so one statement can reach the function through both the id and the name branch. UNION ALL would emit a duplicate edge for it. Declares the resulting `[[0, 1]]` key on both the raw view and the materialized view desc so they match what the planner derives, and adds a test covering the dual spelling.

Review feedback: the doc read as if cut off. Say what the row is.

Review feedback: the comment said mz_object_dependencies where it meant mz_object_dependencies_raw, the view whose own edges the second generator pass appends.

Review feedback: the recovery of function edges by (schema, name) hinges on how the resolver prints function references. Document the explanation.

Review feedback: main is at 26.41.0-dev.0 since the 26.40 cut, so this change ships in 26.41.0.

Review feedback: the 11 -> 12 expectation change is a main-side flake in nightly SLT (2 replicas), not an effect of this PR, and belongs in a separate fix if it persists.

Review feedback: drop the stray double space after the comment marker.

Review feedback: cover each remaining edge-producing path creatable in SLT. A user materialized view; an index whose key calls a function, a custom LIST type, a table created from a source. Also cover dropping each object, asserting no user edge outlives its object.

SangJunBak · 2026-09-03T21:19:19Z

Was noticing a nightly failure related to my PR https://buildkite.com/materialize/nightly/builds/18208#01a06799-8273-49ce-9e13-08cd67cfb039 that didn't appear on nightlies on main. However on running a nightly on main with that exact seed, I was able to reproduce the error https://buildkite.com/materialize/nightly/builds/18221. Going to continue to merge since it's not due to this change but I think this may be related to Metric Sinks. Created a separate ticket here https://linear.app/materializeinc/issue/SQL-680/metric-sink-boot-re-render-registers-a-duplicate-collector-silently

SangJunBak changed the title ~~sql-parser: add a collector for catalog item references~~ sql-150: Convert mz-object-dependencies Aug 17, 2026

SangJunBak force-pushed the jun/convert-mz-object-dependencies branch 7 times, most recently from 8332347 to 730f630 Compare August 28, 2026 02:09

SangJunBak added the ci-nightly PR CI control: also trigger Nightly label Aug 28, 2026

SangJunBak changed the title ~~sql-150: Convert mz-object-dependencies~~ sql-499: Convert mz-object-dependencies Aug 28, 2026

SangJunBak force-pushed the jun/convert-mz-object-dependencies branch from 730f630 to 70ea63b Compare August 28, 2026 07:56

SangJunBak requested a review from mtabebe August 28, 2026 07:56

SangJunBak changed the title ~~sql-499: Convert mz-object-dependencies~~ sql-499: Convert mz_object_dependencies Aug 28, 2026

SangJunBak marked this pull request as ready for review August 28, 2026 08:15

SangJunBak requested review from a team as code owners August 28, 2026 08:15

SangJunBak force-pushed the jun/convert-mz-object-dependencies branch from 70ea63b to 6b64487 Compare August 28, 2026 15:22

SangJunBak requested a review from a team as a code owner August 28, 2026 15:22

mtabebe reviewed Aug 28, 2026

View reviewed changes

ggevay reviewed Aug 31, 2026

View reviewed changes

SangJunBak force-pushed the jun/convert-mz-object-dependencies branch from 6b64487 to 9fa5431 Compare September 1, 2026 18:08

SangJunBak requested review from ggevay and mtabebe September 1, 2026 18:13

ggevay approved these changes Sep 2, 2026

View reviewed changes

sql-parser: add a collector for catalog item references

efe12c7

Walks a raw statement AST and buckets every catalog item reference. CTE bindings are excluded.

SangJunBak force-pushed the jun/convert-mz-object-dependencies branch from 9fa5431 to 61131e6 Compare September 2, 2026 19:06

SangJunBak added 14 commits September 2, 2026 20:06

sql-parser: explain why CTE names are excluded from item references

4e8f6ba

Review feedback: state why a name bound by WITH must not be recorded, and that the exclusion mirrors the name resolver's scoping rules.

sql-parser: make the reference collector's hand-rolled visits drift-p…

a5b423d

…roof The `visit_function` and `visit_query` destructures its node exhaustively so that a field added to `Function` or `Query` fails compilation here instead of being silently skipped.

catalog: finish the BuiltinEdgeRow doc comment

7af49e4

Review feedback: the doc read as if cut off. Say what the row is.

catalog: fix which view the self-edges comment names

124eda8

Review feedback: the comment said mz_object_dependencies where it meant mz_object_dependencies_raw, the view whose own edges the second generator pass appends.

catalog: document why joining function references by name is sound

33f1e1a

Review feedback: the recovery of function edges by (schema, name) hinges on how the resolver prints function references. Document the explanation.

adapter: move the mz_object_dependencies migration step to 26.41.0-dev.0

bb004a6

Review feedback: main is at 26.41.0-dev.0 since the 26.40 cut, so this change ships in 26.41.0.

test: revert unrelated dataflow-count bump in attribution SLT

c1ef635

Review feedback: the 11 -> 12 expectation change is a main-side flake in nightly SLT (2 replicas), not an effect of this PR, and belongs in a separate fix if it persists.

parallel-workload: fix comment indentation nit

7309312

Review feedback: drop the stray double space after the comment marker.

SangJunBak force-pushed the jun/convert-mz-object-dependencies branch from 61131e6 to 8fe8e4a Compare September 2, 2026 20:26

SangJunBak merged commit 7053f0b into main Sep 3, 2026
333 of 337 checks passed

SangJunBak deleted the jun/convert-mz-object-dependencies branch September 3, 2026 21:19

Uh oh!

Conversation

SangJunBak commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Verification

Uh oh!

linear-code Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtabebe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggevay left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SangJunBak commented Aug 17, 2026 •

edited

Loading

linear-code Bot commented Aug 17, 2026 •

edited

Loading

1. MEDIUM -- soft assert on user input in `mz_internal.parse_catalog_item_references`