[catalog] convert mz_sources table to materialized view#36674
Conversation
mtabebe
force-pushed
the
ma/convert-builtins/mz-sources
branch
2 times, most recently
from
a5483b4 to
b17477f
Compare
mtabebe
requested review from
SangJunBak and
aljoscha
and removed request for
a team
def-
left a comment
def-
left a comment
There was a problem hiding this comment.
Do we intentionally remove the progress subsource type? If so we should remove it from the documentation too. If not, we should keep it.
Full nightly will be required for this PR, triggered: https://buildkite.com/materialize/nightly/builds/16516 Lots of related failures, both about progress type and the now missing cluster_id
| "mz_secrets", | ||
| ), | ||
| MigrationStep::replacement( | ||
| "26.26.0-dev.0", |
There was a problem hiding this comment.
Should be 26.27.0-dev.0 I assume
SangJunBak
left a comment
SangJunBak
left a comment
There was a problem hiding this comment.
Had a concern that might be a bug. Let me know if it's a valid concern!
| data->>'kind' = 'GidMapping' AND | ||
| data->'key'->>'object_type' = '2' | ||
| ), | ||
| builtin_sources AS ( |
There was a problem hiding this comment.
Given mz_builtin_sources is essentially a hardcoded list of all the builtin logs/sources created on startup, won't we panic/get in an invalid state when its definition changes but someone forgets to put in a builtin item migration for mz_sources? I'm thinking of the following case:
- Someone is on this version and now they have mz_builtin_sources and the new mz_sources mv
- On the next version, we add a new builtin source. Now mz_builtin_sources' definition changes to include that new source
- The developer doesn't think to create a MigrationStep::replacement for mz_sources since it's not obvious we need a migration
- Because mz_sources is persisted as a builtin materialized view, environmentd expects its definition/fingerprint to be the same
There was a problem hiding this comment.
Oooh that is a really great point. I hadn't considered that... I think the way to avoid this is to build the SQL values directly and that will trigger a fingerprint mismatch, which will make environmentd panic. So it will/should be noticeable
Problem: mz_sources was a builtin table populated imperatively in Rust code in builtin_table_updates.rs. Ideologically we want to move these to views over the catalog. Solution: Convert mz_sources into a materialized view that reads directly from mz_catalog_raw. User sources are derived by parsing their stored create_sql. Builtin and log sources, which don't appear in mz_catalog_raw, are exposed via a new internal view mz_internal.mz_builtin_sources Testing: - Updated information_schema_tables.slt to expect MATERIALIZED VIEW for mz_sources - Added mz_builtin_sources to mz_internal.slt and mz_catalog_server_index_accounting.slt - updated catalog.td to move mz_sources from the tables section into the materialized views section. Part of SQL-118. Port of MaterializeInc#36136 (adapted for the split builtin file structure and current OID range).
mtabebe
force-pushed
the
ma/convert-builtins/mz-sources
branch
3 times, most recently
from
9cef36a to
f1ea73b
Compare
…iltin fingerprint - parse_catalog_create_sql: detect progress subsources and extract of_source_id; use CROSS JOIN LATERAL to call it once per row - Inline builtin sources as VALUES in mz_sources SQL so adding a builtin source changes the fingerprint and forces a migration
mtabebe
force-pushed
the
ma/convert-builtins/mz-sources
branch
from
f1ea73b to
85f1866
Compare
3 participants
Problem:
mz_sources was a builtin table populated imperatively in Rust code in builtin_table_updates.rs.
Ideologically we want to move these to views over the catalog.
Solution:
Convert mz_sources into a materialized view that reads directly from mz_catalog_raw. User sources are derived by parsing their stored create_sql. Builtin and log sources, which don't appear in mz_catalog_raw, are exposed via a new internal view mz_internal.mz_builtin_sources
Testing:
Part of SQL-118.
Port of #36136 (adapted for the split builtin file structure and current OID range).