Repository navigation
Convert mz_materialized_view_refresh_strategies to a builtin mv - #39597
Conversation
|
@ggevay Added you as a reviewer since you seemed like the best to review it! I know we discussed evaluating the expression in a sql function but it gets trickier since we can also have expressions that evaluate functions (e.g. |
| DROP MATERIALIZED VIEW mv_retract | ||
|
|
||
| query I | ||
| SELECT count(*) FROM mz_internal.mz_materialized_view_refresh_strategies WHERE materialized_view_id LIKE 'u%' |
There was a problem hiding this comment.
These counts only look at user views, so no PR-CI test checks the rows of the new builtin_strategies branch: catalog_server_explain.slt only pins its plan, and test/cluster's check_introspection runs only in Nightly. Dropping that branch, or losing a single builtin row, still passes this file. mz_history_retention_strategies.slt already has the same check for its relation. This version passes on the PR's CI image and fails when flipped to 1:
# Every materialized view, builtin or user, has a refresh strategy row.
query I
SELECT count(*)
FROM mz_catalog.mz_materialized_views mv
FULL OUTER JOIN mz_internal.mz_materialized_view_refresh_strategies rs ON mv.id = rs.materialized_view_id
WHERE mv.id IS NULL OR rs.materialized_view_id IS NULL
----
0
|
|
||
| /// Rejects times that `mz_materialized_view_refresh_strategies` could not pack as a | ||
| /// `timestamptz`, whose range is far smaller than `mz_timestamp`'s. | ||
| /// Rejects times that are too large. |
There was a problem hiding this comment.
Nit: the deleted packer was why this check was added, but the bound still matters. With replica expiration on, compute rounds a dataflow's expiration up to the next refresh and logs it through mz_ore::now::to_datetime (compute_state.rs:811), which panics past chrono's range, the same range this check enforces. So it keeps REFRESH AT times out of that panic. Could the doc say so? The comment at materialized_views.slt:765 ("which is how these get packed") is stale too.
| // - aligned_at (timestamptz) -> aligned_to_expression (text) | ||
| // - at (timestamptz)-> at_expression (text) |
There was a problem hiding this comment.
Nit: aligned_at should be aligned_to (also in the description).
| // - aligned_at (timestamptz) -> aligned_to_expression (text) | |
| // - at (timestamptz)-> at_expression (text) | |
| // - aligned_to (timestamptz) -> aligned_to_expression (text) | |
| // - at (timestamptz) -> at_expression (text) |
ea833b7 to
1a4d69a
Compare
1a4d69a to
5330841
Compare
QA LLM Review1. MEDIUM -- In-place upgrade from any pre-merge dev build crash-loops environmentd
Retyping DetailsReproduced with docker, keeping the same
The earlier conversions ( There are three ways out. Keep the desc evolvable, for example by leaving the old columns in place and appending the text columns as new nullable trailing columns. Land a fix for the same-version and forced dev paths before this PR. Or have the conversion allocate a fresh shard when the desc is incompatible, instead of carrying the table's mapping over. |
As mentioned above, this is an instance of
Btw. fixing the underlying issues seems not too hard (but also not urgent), see https://linear.app/materializeinc/issue/SQL-663/dev-to-dev-upgrades-at-the-same-version-skip-all-builtin-schema#comment-f76ac321 |
5330841 to
8bdffff
Compare
For materialized views, we parse the REFRESH values from it such that we can populate `mz_materialized_view_refresh_strategies` in the future. We take the full stable ast string, meaning we don't evaluate expressions stored in the `create_sql` of a statement. This is because we would need the planner and optimizer to evaluate it, something too heavyweight for a SQL function. The consequence of this is we can't reliably report back timestamptz values `mz_materialized_view_refresh_strategies` and return back the full expression string instead. Given we'll be deprecating this feature in the future and `mz_materialized_view_refresh_strategies`'s schema isn't stable, this is acceptable.
…a materialized view We use parse_catalog_create_sql to get the refresh strategy information from the previous commit.
…gies as a materialized view
8bdffff to
2321960
Compare
3 participants
mz_materialized_view_refresh_strategiesto a builtin materialized view. Read commit messages for more detailsaligned_atandatfrom timestamptz to string values.Motivation
Closes SQL-701
Verification
mz_materialized_view_refresh_strategiesalready. Added a retraction test case