Repository navigation
[pull] main from MaterializeInc:main - #1320
Merged
Merged
Conversation
) `UnionBranchCancellation` cancels two branches only when they are direct siblings of the same `Union` and structurally equal (see `compare_branches`). A `Get x` sitting under a `Map` next to a `Union` one level down does not qualify, even when that nested union holds a matching `Negate (Get x)`. The transform ran only in the logical optimizer and the logical cleanup pass, so a plan that reaches the flat sibling shape later kept the cancellable union all the way into the optimized plan. The shape becomes cancellable inside `fixpoint_physical_01`. `LiteralLifting` hoists a literal `Map` that wrapped *both* branches out of the union, turning `union(map(k, x), map(k, union(negate(x), y)))` into `map(k, union(x, union(negate(x), y)))`, and flattening that union-of-union makes `x` and `negate(x)` siblings for the first time. `LiteralLifting` also runs in `fixpoint_logical_02`, but `logical_cleanup_pass` never repeats it, so no earlier `UnionBranchCancellation` call gets to see the flattened form. The transform logic is correct; the gap is pipeline coverage. This change runs `fusion::union::Union` followed by `UnionBranchCancellation` after the final `RelationCSE` in the physical optimizer. The explicit union fusion establishes the flat shape on purpose instead of relying on `FoldConstants`, which flattens a union-of-union only as a side effect of decomposing a `Union` and rebuilding it with `union_many`. The trailing `fold_constants_fixpoint` then fuses the resulting empty constants and `NormalizeLets` drops the now unused binding. Both calls sit behind `enable_union_cancellation_after_relation_cse`, cluster-scoped and default on, so the optimization can be switched off for a user who hits an unexpected regression. Cleanup of the flag is tracked in [STG-41](https://linear.app/materializeinc/issue/STG-41). The motivating cases are a query in `test/sqllogictest/joins.slt` and the second query in `test/sqllogictest/transform/demand.slt`, both of which previously left `union(get l0, negate(get l0), constant)` in the plan and now fold to a constant fast path. The `demand.slt` query no longer reaches a plan that could carry a `dummy`, so it stops covering the scenario its first comment paragraph describes; the comment now records that. Every `test/sqllogictest` file containing `EXPLAIN` (137 files, excluding the upstream `sqlite` and `cockroach` trees) passes unchanged apart from those two goldens. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
### Motivation
`bounded staleness <duration>` shipped in v26.29 behind
`enable_bounded_staleness_isolation`, which has been defaulting on
since. The
level is documented as public preview and still carries the gating
machinery
that a preview needs. This graduates it: the flag goes away and the docs
drop
the preview annotation, so bounded staleness is a supported part of the
`transaction_isolation` surface.
### Description
* Removes `enable_bounded_staleness_isolation` from the feature-flag
table.
`check_transaction_isolation_feature_flag` now gates only `strong
session
serializable`, which is still flagged behind `enable_session_timelines`.
The
shared check and the session-default scrub in `command_handler`
therefore
stay in place; only the bounded staleness arm and the comment's example
change.
* An environment that persisted the removed parameter via `ALTER SYSTEM`
is
unaffected: an unknown system parameter in catalog storage is warned
about
and ignored at startup (`catalog/apply.rs`, `catalog/open.rs`).
* Drops `{{< public-preview />}}` and the **Public preview.** cell from
the
isolation-level reference. The `{{< if-released "v26.29" >}}` guards are
kept,
since older Self-Managed versions still lack the level entirely.
* Parallel workload no longer gates `BoundedStalenessReadAction` on the
flag or
flips the flag, and the flag leaves the LaunchDarkly consistency
allowlist
(it never had an LD flag, so it was listed under
`KNOWN_MISSING_FROM_LD`).
Not included: a release-note entry in
`doc/user/content/releases/_index.md`.
That file has no section for the in-flight version, so the GA note
belongs with
the release-notes commit for whichever release this lands in.
### Verification
* `test/sqllogictest/bounded_staleness.slt` loses the feature-flag
sections and
keeps the role-default coverage: `ALTER ROLE ... SET` of the level, and
a
fresh session picking it up as its starting isolation.
* `test/testdrive/bounded-staleness.td` no longer enables the flag; the
happy-path coverage is otherwise unchanged.
* The `mz-sql` unit test for the shared isolation gate now exercises
`strong
session serializable` (rejected with the flag off, accepted with it on)
and
asserts bounded staleness passes ungated.
Release note: This release makes the bounded staleness isolation level
generally available.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01PfeCp5oXaZ9RayUSdD5aJb)_
Co-authored-by: Claude <noreply@anthropic.com>
### Motivation `mz` fails on read-only commands (e.g. `mz sql`) when `mz.toml` sits on a read-only mount, such as a sandbox that bind-mounts the config file read-only. The command never intends to write anything, so this is an unnecessary failure. ### Description `ConfigFile::load` always opened `mz.toml` with `write(true).create(true)`, even though every mutation (`add_profile`, `remove_profile`, `set_param`, `set_profile_param`) writes the file independently afterward via `fs::write`. The write-mode open at load time was only ever needed to create the file when missing. `load` no longer writes to the file system at all. A missing file loads as an empty configuration, and the first mutation creates whatever is missing. Since `fs::write` creates the file but not its parent directory, all mutations now go through a private `ConfigFile::write` helper that calls `create_dir_all` first, taking over the directory creation that `load` used to perform. Read-only commands therefore work both when `mz.toml` sits on a read-only mount and when it does not exist yet. Dropping the write-mode open changes when an unwritable configuration is reported. `mz profile init` creates a remote app password, and on macOS a keychain entry, before `add_profile` reaches its write. Failing only at that write would leave a credential behind that no profile records, so `ConfigFile::ensure_writable` establishes write access and `init` calls it before logging in. The check creates the parent directory and opens the file with `create(true)`, because whether a missing file can be written depends on the parent hierarchy and no probe settles that as reliably as performing the creation the later write needs anyway. Unit tests cover loading a missing file, loading a read-only file, a missing file inside a read-only directory, and the parent directory creation that moved out of `load`. The permission-dependent assertions skip themselves when the file system does not enforce the permission bits for the process, as is the case under root. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Problem:
A `map[text=>text]` has no per-value nullability, so a metric sink's
`labels` map can hold nulls. Such a row hit `extract_row`, which
unwrapped the
value as a string and panicked the worker. That takes down clusterd, and
the
sink re-renders over the same persisted row on restart, crash-looping
the whole
cluster.
Separately, `drop_optimizer_notices` handled only `Index` and
`MaterializedView`, so a dropped sink's notices were never retracted.
Solution:
Skip a row whose label set is not representable, counting it in
`mz_compute_metric_sink_skipped`. A null value has nothing to encode,
and an
empty string is not a stand-in either, since Prometheus reads it as
absent and
would fold `{a => ''}` into `{}`.
Retract a dropped sink's notices through a new `dataflow_metainfo_mut`,
the
mutable twin of the existing `dataflow_metainfo` getter, shared by both
drop
sites.
Testing:
- unit tests
- a sqllogictest over a filtered, indexed view that exercises both the
panic and the notice retraction on `DROP METRIC SINK`.
Closes: [SQL-645](https://linear.app/materializeinc/issue/SQL-645)
Problem: A builtin schema migration using the `Replacement` mechanism hands the new deployment a fresh persist shard. Nothing writes that shard while the deployment is read-only, because `ComputeController::allow_writes` no-ops in read-only mode, so the MV's write frontier never advances and the 0dt readiness gate has to drop it and everything downstream of it from the caught-up check. The deployment then promotes with those collections unhydrated, and they all hydrate at once at cut-over, spiking catalog-server CPU and degrading catalog queries. Solution: The replacement shard is exclusively owned by this deployment, so the MV can write it while we are still read-only. Do that, and keep the MV in the readiness gate, so it hydrates before cut-over instead of at it. Previously we didn't do this for builtins derived from mz_catalog because catalog shard frontier was not held. However, as of 26.17 it is, so gate the behaviour on the version of the old leader. Testing: - New environmentd integration test `test_0dt_migrated_builtin_mv_hydrates_before_promotion` boots a read-only generation with a forced `replacement` migration and asserts the migrated builtin MVs `mz_databases` and `mz_clusters` are readable before it reports `ReadyToPromote`, proving they hydrate before cut-over rather than at it. - Extended the `0dt` mzcompose workflow `builtin-schema-migrations-replacement` to read those MVs from the read-only generation before promotion, guarding the same invariant through a real cross-version upgrade.
## Motivation Partitioned PostgreSQL tables come up repeatedly with customers and we have no docs for them. PostgreSQL expands a partitioned table to its leaf partitions in a publication, and the PostgreSQL source does not support `publish_via_partition_root`, so users need a documented strategy. Adds a guide under **Ingest data → PostgreSQL**, plus a "Partitioned tables" entry in the PostgreSQL source considerations that warns against `publish_via_partition_root` and links to it. ## What's covered **Approach 1 — ingest the leaf partitions.** One table per partition, unioned into a *materialized* view so that partitions can be added and removed with `CREATE REPLACEMENT MATERIALIZED VIEW` / `ALTER MATERIALIZED VIEW ... APPLY REPLACEMENT`, without recreating dependent objects. (Per @sjwiesman's and Nate Stewart's suggestion — using a plain view means every rollover breaks downstream objects.) **Approach 2 — flatten onto a dedicated replica.** Written as an extension of the existing [dedicated replica guide](https://materialize.com/docs/ingest-data/postgres/logical-replica/) rather than a duplicate: only the partition-specific deltas (`publish_via_partition_root = true` on the primary→replica hop, flat table with the parent's primary key on the replica, mirroring retention with `DELETE`). It also explains *how* the flattening actually happens, since that was the least obvious part in review: logical replication matches publisher to subscriber by schema-qualified name, and `publish_via_partition_root` changes which name goes on the wire, so partitions are never named and the subscriber's table can be an ordinary one. ## Please verify before merge Three claims come from local testing (Postgres 17 → Materialize v26.37.0), not from existing docs. Flagging them so review effort lands in the right place: 1. **"Materialize does not support `publish_via_partition_root` ... can produce incorrect results."** Reproduced locally: the source is created and appears to work, then diverges once partitions are attached/detached, and an `ATTACH` of a populated partition followed by an `UPDATE` of one of its rows produced a negative accumulation (`Non-positive multiplicity in DistinctBy`). Matches [SS-384](https://linear.app/materializeinc/issue/SS-384) and [SS-394](https://linear.app/materializeinc/issue/SS-394). **Verify:** is "can produce incorrect results" the wording we want, and should this instead be a hard error at `CREATE SOURCE`? 2. **The add/remove-partition runbooks.** Verified end to end on v26.37.0, including that dependent materialized views survive both operations and that retiring a partition in Materialize before dropping it upstream retracts the rows rather than stranding them. **Verify:** the ordering in "Remove a partition" is the sequence we want to recommend. 3. **`REPLICA IDENTITY FULL` does not cascade to partitions**, and a partition with neither a replica identity nor a primary key causes PostgreSQL to reject `UPDATE`/`DELETE` against it *on the primary*. Verified on PostgreSQL 17. ## Open question Approach 1 leans on **replacement views, which are Public Preview** and currently annotated "only recommended for use in development environments." That sits awkwardly with recommending them as the primary path in an ingest guide. Options: ship as-is with the preview annotation (current state), soften Approach 1 to plain views and note replacement views as the better option once GA, or lead with Approach 2. Happy to take direction here. ## Testing - `hugo` builds clean; all shortcodes expand and internal links resolve. - Reviewed against `doc/user/.prompts/docs_pre_pr_review.md`. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
### Motivation Spilled chunk bodies currently compress with lz4 unconditionally. Fresh chunks (generation depth 0) are usually consumed by their first merge moments after they spill, so compressing them buys little swap savings and pays encode plus decode on the hottest path. The August benchmark campaign measured this as roughly half of the spill-on co-tenant tail at 50cc: sweeping the floor from compress-everything to never-compress cut the co-tenant probe p99 from 16.6 s to 7.5 s under an ~18k rows/s upsert load (details in the campaign notes, results reproduced across two runs). This adds a generational compression floor: bodies below `column_chunk_compress_min_depth` (default 1, replica-scoped) spill through a new identity codec in `mz_ore::pool`. They stay budget-accounted and swap-backed, but encode and decode reduce to copies. Keeping young bodies out of the pool entirely was rejected because exempted bytes would be invisible to the budget. ### Tips for reviewer * The codec choice happens once at spill time (`codec_for_depth`), so a flag change affects newly spilled bodies only. * `0` restores compress-everything, `255` is effectively never-compress. ### Checklist - [ ] This PR has adequate test coverage / QA involvement has been duly considered. ([trigger-ci for additional test/nightly runs](https://trigger-ci.dev.materialize.com/)) - [ ] This PR has an associated up-to-date [design doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md), is a design doc ([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)), or is sufficiently small to not require a design. - [ ] If this PR evolves [an existing `$T ⇔ Proto$T` mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md) (possibly in a backwards-incompatible way), then it is tagged with a `T-proto` label. - [ ] If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label ([example](MaterializeInc/cloud#5021)). - [ ] If this PR includes major [user-facing behavior changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note), I have pinged the relevant PM to schedule a changelog post.
#38254) ### Motivation A seal walks every stashed chunk to partition updates against the frontier, and for a spilled chunk that walk previously loaded and decoded the whole body even when no update in it could possibly be extracted (or when all of them must be). During sustained ingest the stash backlog is mostly frontier-disjoint chunks, so per seal this decoded the entire backlog to move one boundary chunk. The August benchmark campaign measured the two stash patches together as a 35% mean and ~2x p90 reduction in co-tenant probe latency at 50cc. Spilled bodies now carry their time bounds as resident metadata (an antichain lower bound and the maximal time elements). `extract_into` uses them for two whole-chunk fast paths: ship the chunk untouched when the frontier is past all of its times, keep it untouched (folding its lower bound into the residual frontier) when none of its times are ready. Only chunks the frontier genuinely straddles are loaded, decoded, and split. ### Tips for reviewer * The residual-frontier fold is the subtle part for partial orders: a kept chunk contributes its lower bound, not its individual times, which is sound because extraction only needs an upper bound on what remains. * Both fast paths have dedicated tests, including a frontier-disjoint pass-through case. ### Checklist - [ ] This PR has adequate test coverage / QA involvement has been duly considered. ([trigger-ci for additional test/nightly runs](https://trigger-ci.dev.materialize.com/)) - [ ] This PR has an associated up-to-date [design doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md), is a design doc ([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)), or is sufficiently small to not require a design. - [ ] If this PR evolves [an existing `$T ⇔ Proto$T` mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md) (possibly in a backwards-incompatible way), then it is tagged with a `T-proto` label. - [ ] If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label ([example](MaterializeInc/cloud#5021)). - [ ] If this PR includes major [user-facing behavior changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note), I have pinged the relevant PM to schedule a changelog post.
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 : )