Skip to content

Refactor out inlined values in builtin materialized views - #39468

Open
SangJunBak wants to merge 7 commits into
mainfrom
jun/sql-653-static-builtin-values-in-mv-fingerprint
Open

SangJunBak wants to merge 7 commits into
mainfrom
jun/sql-653-static-builtin-values-in-mv-fingerprint

Conversation

@SangJunBak

Copy link
Copy Markdown
Contributor

I'd recommend this PR commit by commit. All of this is entirely a refactor and most is just moving code around.

Motivation

Closes sql-653

Description

Should have no customer facing changes. The biggest change is instead of inlining builtin values into the fingerprint, we can use our original pattern of using upstream "mz_builtin_*" views. Initially we thought we had to inline the builtin VALUES into the materialized view, otherwise a silent change to an upstream view (i.e. mz_builtin_indexes) would silently create an error in the materialized view. However we've since realized that as long as the schema of the upstream view stays the same, any updates to it will simply sink into the materialized view. If the schema were to change however, e.g. we ASSERT NOT NULL on the mv but have the upstream view provide a NULL value, then the mv would error when queried. However, we have tests that actually query each builtin so this regression would be caught.

Verification

A proof point of this working is mz_object_dependencies. It is an mv that references an upstream view mz_object_dependencies_raw that's constantly changing. This has been in production for a while but things are healthy due to what I stated prior.

@SangJunBak
SangJunBak added this pull request to stack #38849 October 2, 2026 00:20
@SangJunBak
SangJunBak requested a review from a team as a code owner October 2, 2026 00:20
@linear-code

linear-code Bot commented Oct 2, 2026

Copy link
Copy Markdown

SQL-653

@SangJunBak SangJunBak changed the title catalog: extract builtin index rows of mz_indexes into mz_builtin_ind… Refactor out inlined values in builtin materialized views Oct 2, 2026
@SangJunBak
SangJunBak requested review from mtabebe and removed request for mtabebe October 2, 2026 00:21
@SangJunBak SangJunBak added the ci-nightly PR CI control: also trigger Nightly label Oct 2, 2026
@SangJunBak
SangJunBak force-pushed the jun/sql-653-static-builtin-values-in-mv-fingerprint branch from 3db1f61 to 03701c4 Compare October 2, 2026 03:32
@SangJunBak
SangJunBak requested a review from a team as a code owner October 2, 2026 03:32
@SangJunBak SangJunBak removed the ci-nightly PR CI control: also trigger Nightly label Oct 2, 2026
@SangJunBak
SangJunBak force-pushed the jun/sql-653-static-builtin-values-in-mv-fingerprint branch from 03701c4 to 631ddf2 Compare October 2, 2026 04:40
@SangJunBak
SangJunBak force-pushed the jun/sql-653-static-builtin-values-in-mv-fingerprint branch 2 times, most recently from 0ce5fa8 to a3430b2 Compare October 5, 2026 16:13
Base automatically changed from jun/sql-499-convert-item-metadata-tables-to-catalog-views to main October 6, 2026 03:01
…exes

- We need to extract this into its own view such that later, we can extend and reuse mz_builtin_indexes without changing the schema of mz_indexes since it's an mz_catalog relation and stable
- Initially we thought we had to inline the builtin VALUES into the materialized view, otherwise a silent change to an upstream view (i.e. mz_builtin_indexes)  would silently create an error in the materialized view. However we've since realized that as long as the schema of the upstream view stays the same, any updates to it will simply sink into the materialized view. If the schema were to change however, e.g. we ASSERT NOT NULL on the mv but have the upstream view provide a NULL value, then the mv would error when queried. However, we have tests that actually query each builtin so this regression would be caught.
…indexes

- Because we don't need to inline VALUES into the DDLs of our builtin materialized views, we can create a new mz_builtin_log_indexes such that both mz_sources and mz_indexes can query it rather than get sent an iterator of the builtin logs.
- We simply extract inline VALUES code into a new builtin view called mz_builtin_logs then reference it in mz_builtin_indexes
- This comes with the nice benefit of not having to dynamically generate each mz_indexes and define the order statically.
- We can also turn make_mz_indexes into a static MZ_INDEXES like the rest of our builtins
- In a later commit, we'll making the same refactor for mz_sources
- In mz_sources, rather than inline the builtin sources and logs as a VALUES list, read them from mz_builtin_sources
- mz_builtin_sources already existed inside "make_builtin_sources", however it was just never used and was dead code. This is why we delete code in make_mz_sources but don't see a positive diff in this commit. However if you were to look inside make_builtin_sources, the same code we deleted exists there.
…tin_log_indexes

- Move `privileges` to `mz_builtin_log_indexes` from `mz_builtin_sources` 
- Instead of embedding the builtin logs to mz_sources, just join on `mz_builtin_log_indexes`
Simply move make_mz_sources to a static mz_sources to remove an unnecessary function call.
Given we're not sharing iterators across these view generators, we can simplify the code further.
@SangJunBak
SangJunBak force-pushed the jun/sql-653-static-builtin-values-in-mv-fingerprint branch from a3430b2 to b3e9ac9 Compare October 6, 2026 03:02

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant