[catalog] convert mz_indexes to BuiltinMaterializedView#36771
Conversation
mtabebe
force-pushed
the
ma/convert-builtins/mz-indexes
branch
9 times, most recently
from
9315e0d to
0a197ac
Compare
|
One test failure is a failure on main |
| "mz_sources", | ||
| ), | ||
| MigrationStep::replacement( | ||
| "26.27.0-dev.0", |
There was a problem hiding this comment.
(Will need to be updated when rebased on main)
|
|
||
| // Generate mz_indexes with builtin index/log entries inlined as VALUES so | ||
| // that its SQL fingerprint changes whenever a builtin index or log is added or | ||
| // removed, forcing an explicit MigrationStep::replacement. |
There was a problem hiding this comment.
Maybe add a test for this, similar to test_mz_sources_fingerprint_changes_with_new_builtin_source.
| SELECT isi.data->'key'->>'name', mz_internal.parse_catalog_id(isi.data->'key'->'cluster_id') | ||
| ) AS l(idx_name, cluster_id) | ||
| JOIN mz_internal.mz_catalog_raw AS gm ON | ||
| gm.data->>'kind' = 'GidMapping' AND |
There was a problem hiding this comment.
Would be good to also match object_type, like in the earlier make_mz_sources.
| .iter() | ||
| .map(|e| e.to_ast_string_stable()) | ||
| .join(", "); | ||
| let key_exprs_escaped = escaped_string_literal(&key_exprs); |
There was a problem hiding this comment.
Could you add a comment on why this is using escaped_string_literal as opposed to the nearby usages of assert_safe_builtin_name? (This got both me and Claude confused for a moment.) I think it's because the key expressions can legit contain quotes, which we want to actually escape, instead of asserting on them, right?
There was a problem hiding this comment.
Yes, that is correct! It is potentially arbitrary SQL
Problem mz_catalog.mz_indexes is a BuiltinTable whose rows are written by pack_index_update. This requires coordinated writes from a single environmentd process, which conflicts with the multi-envd goal. Solution Convert mz_indexes to a BuiltinMaterializedView backed by a query over mz_internal.mz_catalog_raw. Testing Updated tests to reflect that mz_indexes is now a MATERIALIZED VIEW
mtabebe
force-pushed
the
ma/convert-builtins/mz-indexes
branch
from
0a197ac to
afacd83
Compare
2 participants
Problem
mz_catalog.mz_indexes is a BuiltinTable whose rows are written by pack_index_update. This requires coordinated writes from a single environmentd process, which conflicts with the multi-envd goal.
Solution
Convert mz_indexes to a BuiltinMaterializedView backed by a query over mz_internal.mz_catalog_raw.
Testing
Updated tests to reflect that mz_indexes is now a MATERIALIZED VIEW
Remove these sections if your commit already has a good description!