catalog: convert mz_materialized_views into a view over the catalog#35819
catalog: convert mz_materialized_views into a view over the catalog#35819teskje wants to merge 3 commits intoMaterializeInc:mainfrom
Conversation
|
Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone. PR title guidelines
Pre-merge checklist
|
This commit adds the first constant builtin view exposing builtin objects to the catalog. Specifically, `mz_builtin_materialized_views` reports what builtin materialized views exist. It is required to define `mz_materialized_views` as a view over the catalog, since the catalog does not contain builtin objects. Further builtin views exposing other builtin object types will be added as needed to support converting more builtin tables.
f27e0e4 to
0ae57d9
Compare
| // TODO: This function isn't parsing JSONB and therefore shouldn't live in the `jsonb` module. | ||
| // Consider moving all the `parse_catalog_*` functions into their own module. |
There was a problem hiding this comment.
Didn't want to include this in this PR because of the noise. I'll make a separate code movement PR.
|
|
||
| use mz_repr::{Datum, InputDatumType, OutputDatumType, ReprColumnType, RowArena, SqlColumnType}; | ||
|
|
||
| use crate::scalar::func::RedactSql; |
There was a problem hiding this comment.
I put this in mz_expr::scalar::func so it's next to pretty_sql, which is the only similar function we have, afaict. But now it's the only unary func that doesn't live in mz_expr::scalar::func::impls. Should I move it?
There was a problem hiding this comment.
I'd say it's ok. We can move it later if needed.
|
|
||
| new_mz_tables_gid = c.sql_query( | ||
| "SELECT id FROM mz_tables WHERE name = 'mz_tables'", | ||
| service="mz_new", | ||
| )[0][0] | ||
| new_mv_gid = c.sql_query( | ||
| "SELECT id FROM mz_materialized_views WHERE name = 'mv'", | ||
| service="mz_new", | ||
| )[0][0] | ||
| assert new_mz_tables_gid == mz_tables_gid | ||
| assert new_mv_gid == mv_gid | ||
| # mz_internal.mz_storage_shards won't update until this instance becomes the leader | ||
|
|
There was a problem hiding this comment.
Had to remove this part because mz_materialized_views is now a builtin MV and becomes unreadable in read-only mode after a replacement migration. I think this is fine, the checks here didn't add much anyway.
| .with_key(vec![0]) | ||
| .with_key(vec![2]) | ||
| .with_key(vec![4]) | ||
| .with_key(vec![6]) |
There was a problem hiding this comment.
The optimizer decides these keys based on the concretes values in the VALUES list. It determines that name, definition and create_sql values are keys because they are unique... which isn't wrong!
There was a problem hiding this comment.
Yeah, these are somewhat surprising keys, but I'd say it's ok. (At some point, we'll hopefully introduce more rigorous testing for the keys of builtins.)
|
Green nightly (except that sqlsmith stubles over |
This commit adds two new internal sqlfuncs to support converting `mz_materialzied_views` into a view over the catalog. * `parse_catalog_create_sql` parses a create_sql string and returns information extracted from it as JSON * `redact_sql` redacts a given SQL statement
0ae57d9 to
ddfb2e6
Compare
| .with_key(vec![0]) | ||
| .with_key(vec![2]) | ||
| .with_key(vec![4]) | ||
| .with_key(vec![6]) |
There was a problem hiding this comment.
Yeah, these are somewhat surprising keys, but I'd say it's ok. (At some point, we'll hopefully introduce more rigorous testing for the keys of builtins.)
|
|
||
| new_mz_tables_gid = c.sql_query( | ||
| "SELECT id FROM mz_tables WHERE name = 'mz_tables'", | ||
| service="mz_new", | ||
| )[0][0] | ||
| new_mv_gid = c.sql_query( | ||
| "SELECT id FROM mz_materialized_views WHERE name = 'mv'", | ||
| service="mz_new", | ||
| )[0][0] | ||
| assert new_mz_tables_gid == mz_tables_gid | ||
| assert new_mv_gid == mv_gid | ||
| # mz_internal.mz_storage_shards won't update until this instance becomes the leader | ||
|
|
|
|
||
| use mz_repr::{Datum, InputDatumType, OutputDatumType, ReprColumnType, RowArena, SqlColumnType}; | ||
|
|
||
| use crate::scalar::func::RedactSql; |
There was a problem hiding this comment.
I'd say it's ok. We can move it later if needed.
| // TODO: This function isn't parsing JSONB and therefore shouldn't live in the `jsonb` module. | ||
| // Consider moving all the `parse_catalog_*` functions into their own module. |
ddfb2e6 to
2b78c03
Compare
This PR converts the
mz_materialized_viewsbuiltin table into a materialized view over the catalog.It includes some pre-work to make this possible:
mz_builtin_materialized_viewsthat exposes... builtin materialized views. Themz_materialized_viewsquery joins against that view to augment the catalog contents (which only include user MVs) with all the builtin MVs.parse_catalog_create_sql, to extract information from the MVcreate_sqlredact_sql, to derivedredacted_create_sqlfrom the MVcreate_sqlAlternatives to having
mz_builtin_materialized_viewsas a constant builtin view:mz-expr, while builtins are defined inmz-catalog, and the dependency relationship is the wrong way round. We could introduce a way to add table functions that get evaluated during planning time, but afaict that infrastructure doesn't exist yet.TODO
redact_sql: postgres: exclude theredact_sqlfunction sqlsmith#6Motivation
Part of SQL-118