SQL-150: convert mz_tables and mz_views to materialized views - #38152
SQL-150: convert mz_tables and mz_views to materialized views#38152SangJunBak wants to merge 4 commits into
Conversation
- With temporary items durable in the catalog shard, mz_tables and mz_views can be derived from mz_internal.mz_catalog_raw. Both show every item including temporary ones, matching the previous builtin tables. Temporary rows keep the temporary schema sentinel "0" in schema_id. - Builtin tables and views are reported through two new generated constant views, mz_internal.mz_builtin_tables and mz_internal.mz_builtin_views, following the mz_builtin_materialized_views pattern - For all the mz_builtin_* views, we don't show it's actual SQL definitions in mz_views since it's not possible for mz_builtin_views to show itself and the definitions of the others made the relation unreadable. Not sure if this is the right design decision - parse_catalog_create_sql now exposes 'definition' for views and 'source_id' for tables created from sources.
Updates various tests to acommodate for new builtin objects as well as updates the descriptions of other tests related to temporary objects
mz_tables and mz_views select rows by parse_catalog_create_sql(...)->>'type' and read 'definition' and 'source_id' out of the same call, but the function had no tests. Pins the reported type for every statement kind an Item record can hold, the exact mz_views.definition rendering (which pg_views exposes and which moved here out of the deleted pack_view_update), its idempotence under re-parsing, source_id presence for CREATE TABLE FROM SOURCE and absence for plain and webhook tables, and the four error paths. The error paths matter more than they used to: the MVs call this inside their WHERE clause, so an item the parser rejects makes the whole relation unreadable rather than breaking one row's packing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fe6e19e to
3636e6f
Compare
mz_tables and mz_views became BuiltinMaterializedViews over mz_internal.mz_catalog_raw, deriving every column from durable catalog JSON, but the conversion added no test file. Every earlier conversion (mz_indexes, mz_audit_events, mz_postgres_sources) got one. Follows the established lockdown shape: one section per union branch, plus the temporary-item sentinel schema id, the exactly-once property across the user and builtin branches, and the ASSERT NOT NULL columns. Two checks are independent of the MV rather than golden values: oid is compared against a regclass cast, which resolves through the in-memory catalog, and mz_views.definition is compared against the definition of a view planned from it, so the rendering is verified to be a fixed point end to end. create_sql cannot be compared against SHOW CREATE, which humanizes item ids and pretty-prints by design. Also pins the one known difference from the old builtin tables: mz_builtin_views cannot list itself, so it is the single builtin view absent from mz_views. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3636e6f to
15548b9
Compare
| pub const FUNC_MZ_AWS_CONNECTION_ROLE_ARN_OID: u32 = 17115; | ||
| pub const VIEW_MZ_BUILTIN_TABLES_OID: u32 = 17116; | ||
| pub const VIEW_MZ_BUILTIN_VIEWS_OID: u32 = 17117; | ||
| pub const MV_MZ_TABLES_OID: u32 = 17118; |
There was a problem hiding this comment.
Minor thing: We agreed in #36912 (comment) (re-affirmed on #37725, which also realigned #37679's deviation) to reuse the old OID when converting a builtin table to a materialized view: rename TABLE_MZ_TABLES_OID -> MV_MZ_TABLES_OID keeping 16707, and likewise 16712 for mz_views, instead of minting 17118/17119. That keeps user-visible pg OIDs stable across the conversion, avoids burning OIDs, and shrinks the oid.slt diff. (17116/17117 for the two genuinely new views are of course fine.)
| "26.38.0-dev.0", | ||
| CatalogItemType::MaterializedView, | ||
| MZ_CATALOG_SCHEMA, | ||
| "mz_views", |
There was a problem hiding this comment.
Two things that need to move in lockstep with these steps:
misc/python/materialize/checks/all_checks/builtin_version_pin.pymaps each stablemz_catalogbuiltin to the version that converted it to a view, andmz_tables/mz_viewscurrently sit atNonethere. Could you set both to the release these steps ship in (currentlyv26.38.0)? Nothing goes red when this is missed (every reader on current upgrade paths carries the catalog: tolerate and strip stale builtin version pins on upgrade #37610 tolerance fix), so it only shows up by reading.- The workspace version bump lands Thursdays. If the stack doesn't merge before the 26.39 bump, these steps and the pin entries need to move together.
| # The placeholder rows are exactly the generated views, and nothing else is | ||
| # elided. | ||
| query T | ||
| SELECT name FROM mz_views WHERE definition LIKE '%definition elided%' ORDER BY name |
There was a problem hiding this comment.
This is the slt-3 CI failure: under --auto-index-selects the harness wraps this query in a helper view whose definition contains the literal '%definition elided%', so the helper view matches its own filter and adds an extra v... row. Adding AND id LIKE 's%' (builtin rows only, which is the intent anyway) fixes it.
| /// view. The placeholder also embeds the view's qualified name so that the | ||
| /// `definition` and `create_sql` columns stay unique across rows, which the | ||
| /// declared keys rely on. | ||
| fn make_builtin_views<'a>( |
There was a problem hiding this comment.
On the design question in the description: the placeholder approach seems right to us. Only the mz_builtin_views self-row is a hard impossibility, but giving the other reporters their real SQL would make this view's VALUES embed megabytes (mz_builtin_materialized_views alone carries every builtin MV's definition), and the parseability constraint for redact_sql is handled correctly here. Three small follow-ups:
- The PR description says
mz_builtin_viewsis absent frommz_views, but the code (and the lockdown SLT) list it with a placeholder. The description needs the update, not the code. - This changes what
mz_views/pg_viewsshow for the two pre-existing reporter views (real SQL before, placeholder now), so a sentence in themz_viewsuser docs would be good. - A related cosmetic delta worth stating deliberately: builtin view rows'
create_sql/redacted_create_sqlused to be the raw in-memory text (CREATE VIEW mz_catalog.x AS ..., unquoted idents) and are now stable reprints (quoted).definitionis unchanged either way. Fine by us, just making it a chosen delta rather than an accident.
This PR is meant to be merged with everything else in the stack and is split for review purposes
Motivation
sql-150
Description
catalog: convert mz_tables and mz_views to materialized views
mz_tablesandmz_viewscan be derived frommz_internal.mz_catalog_raw. Both show every item including temporary ones, matching the previous builtin tables. Temporary rows keep the temporary schema sentinel"0"inschema_id.mz_internal.mz_builtin_tablesandmz_internal.mz_builtin_views, following themz_builtin_materialized_viewspattern.mz_builtin_*views, we don't show their actual SQL definitions inmz_views, since it's not possible formz_builtin_viewsto show itself and the definitions of the others made the relation unreadable. Not sure if this is the right design decision.parse_catalog_create_sqlnow exposes'definition'for views and'source_id'for tables created from sources.test: update tests for mz_tables/mz_views as materialized views
Updates various tests to accommodate the new builtin objects, and updates the descriptions of other tests related to temporary objects.
expr: test parse_catalog_create_sql
mz_tablesandmz_viewsselect rows byparse_catalog_create_sql(...)->>'type'and read'definition'and'source_id'out of the same call, but the function had no tests.Pins:
mz_views.definitionrendering (whichpg_viewsexposes and which moved here out of the deletedpack_view_update)source_idpresence forCREATE TABLE FROM SOURCEand absence for plain and webhook tablesThe error paths matter more than they used to: the MVs call this inside their
WHEREclause, so an item the parser rejects makes the whole relation unreadable rather than breaking one row's packing.test: add mz_tables and mz_views lockdown slts
mz_tablesandmz_viewsbecameBuiltinMaterializedViews overmz_internal.mz_catalog_raw, deriving every column from durable catalog JSON, but the conversion added no test file. Every earlier conversion (mz_indexes,mz_audit_events,mz_postgres_sources) got one.Follows the established lockdown shape: one section per union branch, plus the temporary-item sentinel schema id, the exactly-once property across the user and builtin branches, and the
ASSERT NOT NULLcolumns.Two checks are independent of the MV rather than golden values:
oidis compared against aregclasscast, which resolves through the in-memory catalogmz_views.definitionis compared against the definition of a view planned from it, so the rendering is verified to be a fixed point end to endcreate_sqlcannot be compared againstSHOW CREATE, which humanizes item ids and pretty-prints by design.Also pins the one known difference from the old builtin tables:
mz_builtin_viewscannot list itself, so it is the single builtin view absent frommz_views.Verification