Repository navigation
[pull] main from MaterializeInc:main - #1265
Merged
Merged
Conversation
…ICA (#38061) `CREATE CLUSTER` and `CREATE CLUSTER REPLICA` now accept an `IF NOT EXISTS` clause, matching the 13 other `CREATE` statements that already support it. With it, creating a cluster or replica whose name is already taken succeeds and emits a `cluster "c" already exists, skipping` notice (SQLSTATE 42710) instead of failing, so idempotent provisioning scripts no longer need a pre-flight existence check. Skipping leaves the existing object completely untouched. The configuration in the statement is not applied, so this is not an upsert. Previously these two statements produced a misleading parse error, because `IF` was consumed as the object name and the failure surfaced one token later: ``` CREATE CLUSTER IF NOT EXISTS c (SIZE '25cc') ERROR: Expected one of AUTO or AVAILABILITY or DISK or ..., found NOT ``` ## Implementation The machinery already exists end to end, so this is mostly threading a flag: `if_not_exists` on the two AST nodes and the two plans, a `parse_if_not_exists()` call in each parse function, and the established "turn the catalog's already-exists error into a notice" arm in the sequencer. Two things worth calling out: * The skip happens at the catalog transaction, where every sibling path puts it, rather than short-circuiting early on the in-memory catalog. That means the statement is validated in full first, so `CREATE CLUSTER IF NOT EXISTS c (SIZE 'typo')` still reports the bad size rather than silently succeeding because `c` happens to exist. The cost is that a skipped statement still allocates (and discards) a cluster id, which is a durable catalog write. That is the same cost every other `CREATE ... IF NOT EXISTS` already pays, and silently accepting a typo'd config in a provisioning script is the worse failure mode. * A bare `if` identifier now renders quoted, joining the existing list of keywords quoted for exactly this reason (`PREPARE`, `INTO`, `WHEN`, …). The `IF [NOT] EXISTS` clause sits exactly where the object name goes, so `CREATE CLUSTER if (SIZE …)` rendered in Simple mode no longer reparses. That affects the parser's own display roundtrip test, the user-facing `pretty_sql()` function, and `mz-deploy`, which renders a project file's statement with `to_string()` and executes it. `SHOW CREATE CLUSTER` and the plan/unplan roundtrip assertion in `plan_create_cluster` both use Stable mode, which already quoted everything, so neither was ever affected. The flip side is a grammar break: `CREATE CLUSTER if …` and `CREATE CLUSTER REPLICA if.r …` used to parse, since the parser accepts keywords as identifiers, and now do not. Quoting still works, and `DROP CLUSTER if` was already broken the same way. I smoke tested the upgrade: created a cluster named `if` (unquoted) on `main` along with a table, index and materialized view in it, then restarted on this branch against the same catalog. Bootstrap is clean, the cluster and its objects survive and serve queries, and `ALTER`/`DROP` by quoted name work. The only observable change is that renderings which used to emit a bare `if` now quote it, for example `SHOW CREATE INDEX` goes from `IN CLUSTER if` to `IN CLUSTER "if"`. `SHOW CREATE CLUSTER` does not render `IF NOT EXISTS`. It is a property of the statement, not of the cluster. ## Tests * `src/sql-parser/tests/testdata/ddl`: syntax for both statements, the `IF EXISTS` typo, and a cluster literally named `if`. * `test/sqllogictest/cluster.slt`: idempotence, that the existing object is left untouched, that a bad `SIZE` is still reported, the managed and unmanaged cluster variants, `FEATURES`, reserved names, case sensitivity, full `SHOW CREATE CLUSTER` output, that the replica form still requires the cluster to exist, and that it does not soften the managed-cluster rejection. * `test/pgtest-mz/notice.pt`: the wire-level notice for both statements. Fixes [SQL-601](https://linear.app/materializeinc/issue/SQL-601)
…37921) ### Motivation Part 4 of 7 in a stack that moves `DELETE`, `UPDATE` and `INSERT ... SELECT` off the coordinator onto the session task, using optimistic concurrency control. Design doc: [`20260210_incremental_occ_read_then_write.md`](https://github.com/MaterializeInc/materialize/blob/aljoscha/occ-06-occ-path/doc/developer/design/20260210_incremental_occ_read_then_write.md) (lands in part 6). The group committer picks a write timestamp from the oracle, which is right for a write whose diffs are valid whenever they land. A write computed against a snapshot is not like that. Its diffs are only correct at the frontier they were read at, so the timestamp has to come from the caller and the committer has to be able to refuse it. Closes [SQL-590](https://linear.app/materializeinc/issue/SQL-590) ### Description `TableWriteCmd::TimestampedWrite` carries a caller-chosen timestamp and gets exactly one attempt. An `InvalidUppers` means another writer holds the upper at or past that timestamp, so the snapshot the diffs were computed from is stale, and the answer is `TimestampPassed`. Never a retry at a fresh timestamp: retrying stale diffs later is precisely the lost update this exists to prevent. Deciding what to do about a conflict is the caller's job, and it needs a new snapshot to do it. A concurrent `ALTER TABLE` is caught by a separate guard on the blind path, not here. `UserWriteResponder::Internal` names the generation its diffs were computed against in `WriteTarget`, and `stage_group_commit` answers `TargetChanged` if the table's latest `GlobalId` has moved on. That check runs on the coordinator loop rather than in the committer. `TimestampedWriteRequest` carries pre-resolved `GlobalId`s and has no such check. It relies on its submitter resolving them against the current catalog on the coordinator loop and sending with no await in between, so channel order keeps the append ahead of any `Forget` for the same table. Part 6's `handle_attempt_write` is that submitter. The struct's doc now states this as a contract, because a non-empty append for a table whose write handle is gone trips an assert in the storage controller. Both txns-shard write paths now share `attempt_write_to_txns`, so the catalog upper advance, the metric, the `InvalidUppers` classification and the oracle apply exist once. The two differ only in what they do with a conflict, which is the difference worth seeing. The result travels back through a responder whose `Drop` reports `Indeterminate`. Without that, a dropped sender on shutdown would panic the waiting session task instead of failing its statement. **Temporary scaffolding.** Nothing calls this until part 6, so a `dead_code` allowance on the unreached items keeps the crate warning-free. Part 6 removes it in the same commit that adds the callers. ### Verification No caller yet, so this part is carried by the tests in parts 6 and 7, and by existing coverage showing the shared `attempt_write_to_txns` did not change ordinary group commit behavior.
…ion (#38074) Follow up to incident 1175 where it was observed that the new sink implementation schedules long running blocking work on tokio worker threads causing them to stop scheduling all the other tasks, leading to lost reader leases. This is only to guard self-managed deployments from the problem. In cloud we have already disabled it for everyone via LD. We should consider re-flipping the default after having rolled it out gradually via LaunchDarkly for some time. Signed-off-by: Petros Angelatos <petrosagg@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )