Repository navigation
[pull] main from MaterializeInc:main - #1287
Merged
Merged
Conversation
…8191) `internal_copy_schema_default_privs` interpolated the target schema name raw into `ALTER DEFAULT PRIVILEGES ... IN SCHEMA`, so the parser folded it to lowercase. #38066 started creating the deployment schema with a quoted name, which made the two disagree for any schema name that is not a bare lowercase identifier: `deploy_init` created `"Prod_dbt_deploy"` and then failed trying to alter default privileges in `prod_dbt_deploy`, after the schema already existed. The retry then took the schema-already-exists branch, which skips both privilege-copying macros, so the operation reported success while silently dropping the grants and default privileges the production schema had. Test run: https://buildkite.com/materialize/nightly/builds/18074 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
### Motivation The style bullet in `AGENTS.md` told agents to "avoid em-dashes and semicolons", which read as an absolute ban. That produced a bad loop in practice: write a sentence with a semicolon, strip the semicolon to satisfy the rule, notice the remainder no longer parses, then rewrite the sentence from scratch. The rule was stronger than intended for semicolons. ### Description Split the single bullet in two, so the two marks get different strengths: * Em-dashes: unchanged, still avoid them. * Semicolons: discouraged rather than banned. Prefer a full stop or a comma, but a semicolon is acceptable where splitting genuinely hurts the sentence, and lists needing one for their own punctuation always are. The new bullet also names the failure mode directly, so an agent recognizes the loop instead of grinding through it: aim for prose that never needed the semicolon rather than prose with the semicolon taken back out, because a sentence left mangled by a deleted semicolon is worse than the semicolon. ### Verification Documentation-only change to `AGENTS.md` (`CLAUDE.md` is a symlink to it). No code or tests are affected. --- _Generated by [Claude Code](https://claude.ai/code/session_0127qk44MkFXAyUPEUWhtHP9)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Follow-up to #37976, which still had gaps `Regex::new` projected a byte cost per AST node against a 1 GiB budget, charging a bracketed range like a literal. Case folding walks a range's whole span and keeps one Unicode range per fold mapping, so `[a-\x{2FFF}]` cost 22x its charge: a 1 MiB pattern of them was accepted and allocated 4.7 GiB before erroring, reachable from any `SELECT 'x' ~* <pattern>`. An inline `(?i)` takes the same path, so case-sensitive operators were exposed too. Drop the cost model rather than adding a price for ranges. Per-kind byte prices need calibrating against the pinned `regex-syntax` and fail silently when one is set too low, which is how this got through. Two structural limits replace it: pattern bytes, which bound the node count and every kind whose cost is bounded, and `MAX_REGEX_CHARACTER_CLASSES` (2000), covering Unicode, Perl and POSIX classes and bracketed ranges. `PatternTooExpensive` becomes `TooManyCharacterClasses`. The counter's matches are exhaustive, so a `regex-syntax` bump adding a node kind fails to compile rather than defaulting it to "not a class". The two limits are independent, so the byte limit can now move on its own. It contributes the larger share of the bound, since 1 MiB of literals still translates to roughly 400 MiB. Counting rather than pricing means a pattern is rejected on its class count even where every class is cheap, and that is a deliberate break with released behavior. A machine-generated alternation carrying, say, 3000 `\d` compiles on every released version and does not compile here. Such a pattern inside an existing view const-folds to an error literal, so the object still loads but queries against it error, and plan-time compiles like `regexp_extract` fail to re-plan. The byte limit shipped with the same property, and the shapes that carry thousands of classes are contrived, so this is worth taking. `regex_two_limits_bound_what_a_compile_spends` replaces the per-kind charge table, holding measured translation heap against what the limits allow and pinning how each node kind is classified. `regex_wide_bracketed_range_counts_as_a_class` covers the wide-range vector, and `regex.slt` gains cases for it under both `~*` and an inline `(?i)`. Release notes: This release will reject regular expressions containing more than 2000 character classes, counting each Unicode, Perl, or POSIX class such as `\p{L}`, `\d`, or `[[:alpha:]]`, and each range such as `a-z`. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
<!-- ccr-slack-attribution --> _Requested by **Moritz Hoffmann** · [Slack thread](https://materializeinc.slack.com/archives/C08AENAP691/p1786626884672969?thread_ts=1786626884.672969&cid=C08AENAP691)_ **Before:** a self-managed deployment had arrangement dictionary compression off even on a cluster with `EXPERIMENTAL ARRANGEMENT COMPRESSION` set. The environment-wide gate defaults off, and self-managed has no LaunchDarkly to flip it, so there was no way to turn the feature on at all. **After:** the environment-wide gate defaults on, so the per-cluster `EXPERIMENTAL ARRANGEMENT COMPRESSION` option is the only knob. This also aligns the out-of-the-box default with cloud, where LaunchDarkly already serves `true`. **How:** the compiled-in `default` of the `enable_arrangement_dictionary_compression_alpha` dyncfg (`ENABLE_ARRANGEMENT_DICTIONARY_COMPRESSION_ALPHA` in `src/compute-types/src/dyncfgs.rs`) changes from `false` to `true`. That is the whole functional change; the surrounding doc comment is updated to record why the default is on. The gate is consumed in `ComputeController::add_replica` (`src/compute-client/src/controller.rs`), where it is ANDed with the per-cluster `config.arrangement_compression` to produce `ReplicaConfig::arrangement_dictionary_compression`. The AND is untouched, so a cluster that does not request compression still gets none. **Caveat:** the environment-wide gate is read once at replica creation and is not part of `ReplicaShape`, so existing replicas do not pick up the change until they are recreated (restart, resize, or cluster reconfiguration). ### Motivation Self-managed customers cannot enable arrangement dictionary compression today. The feature is gated by two independent switches that are ANDed together at replica creation: the environment-wide dyncfg `enable_arrangement_dictionary_compression_alpha`, and the per-cluster SQL option `EXPERIMENTAL ARRANGEMENT COMPRESSION`. In cloud the dyncfg is set to `true` through LaunchDarkly, so only the cluster option matters in practice. Self-managed has no LaunchDarkly and therefore falls back to the compiled-in default, which is `false`, leaving the AND permanently false regardless of how the cluster is configured. Flipping the compiled-in default makes the cluster option the single knob everywhere. Requested in the Slack thread linked above. ### Description One-line functional change in `src/compute-types/src/dyncfgs.rs`: ```rust pub const ENABLE_ARRANGEMENT_DICTIONARY_COMPRESSION_ALPHA: Config<bool> = Config::new( "enable_arrangement_dictionary_compression_alpha", true, // was: false "Enable arrangement dictionary compression (alpha; not yet production-ready).", ); ``` The `_alpha` name and the description are deliberately left alone: this changes the default of the gate, not the maturity of the feature, and the flag remains a kill switch that can be set to `false` to disable compression on new or restarted replicas regardless of per-cluster configuration. The doc comment previously asserted "default off" as part of its disposition note, which would now be wrong, so it is corrected and gains a short note on why the compiled-in default is the one that matters for a deployment without LaunchDarkly. Notes for the reviewer: - **Existing replicas are unaffected.** `ComputeController::add_replica` captures the dyncfg once and stores it in `ReplicaConfig`, and the value is not part of `ReplicaShape`, so no reconciliation is triggered by the flip. Only replicas created after the upgrade see the new default. - **The per-cluster option default is unchanged.** `EXPERIMENTAL ARRANGEMENT COMPRESSION` still defaults to `false` (`src/sql/src/plan/statement/ddl.rs`, `experimental_arrangement_compression.unwrap_or(false)`), so no existing cluster starts compressing as a result of this PR. Compression is still strictly opt-in per cluster. - **CI behavior is unchanged.** `misc/python/materialize/mzcompose/__init__.py` pins `enable_arrangement_dictionary_compression_alpha` to `"false"` in the shared mzcompose settings, so the default flip does not silently turn compression on across the mzcompose test suites. That pin is left as-is in this PR; whether it should now be dropped, or flipped to `true` per the repo guidance that feature flags default on in test configuration, is a separate decision. ### Verification No test expectations depend on this default: - `test/testdrive/arrangement-dictionary-compression.td` sets the flag explicitly with `ALTER SYSTEM SET ... = true` before creating its cluster, so it is insensitive to the default. Its trailing `ALTER SYSTEM RESET` now resets to `true` rather than `false`, which is only cleanup at the end of the file. - The `.slt` expectations that mention `EXPERIMENTAL ARRANGEMENT COMPRESSION = false` (`test/sqllogictest/cluster.slt`, `managed_cluster.slt`, `materialized_views.slt`) render the per-cluster option, not the dyncfg, and that option's default is unchanged. - No Rust unit test asserts this dyncfg's default; the `clusterd-test-driver` cases in `src/clusterd-test-driver/src/text.rs` pass the value explicitly. - `misc/python/materialize/parallel_workload/action.py` already flips this flag across both values, so it keeps exercising both paths. `cargo check` could not be run in this environment (a git dependency, `MaterializeInc/async-compression`, is outside the sandbox's authorized repository set, so dependency resolution fails before compiling). `rustfmt --check` on the touched file is clean. The change is a `bool` literal in a `const` initializer plus a doc comment, so compilation risk is nil; CI will confirm. One thing worth a second pair of eyes: `test/launchdarkly-flag-consistency` fails when a flag's production LaunchDarkly default differs from the compiled-in default unless the flag is allowlisted, and `enable_arrangement_dictionary_compression_alpha` currently appears in neither `INTENTIONAL_LD_OVERRIDES` nor `KNOWN_CROSS_ENV_DIVERGENCES`. If production LaunchDarkly serves `true` as its flag default, this PR removes a discrepancy and the check stays green. If cloud instead has it on via a targeting rule while the served default is still `false`, this PR creates a new discrepancy there. Someone with LaunchDarkly access should confirm which it is. ### Note on `launchdarkly-flag-consistency` That check lives in the Nightly pipeline (`ci/nightly/pipeline.template.yml`), not the `test` pipeline, so it did not run on this PR. It compares a flag's compiled-in default against the production LaunchDarkly default and allowlists known divergences. This flag appears in neither `INTENTIONAL_LD_OVERRIDES` nor `KNOWN_CROSS_ENV_DIVERGENCES`. If LaunchDarkly's *served* default is already `true`, this change removes a divergence and Nightly stays green. If cloud is enabled via a targeting rule while the served default is still `false`, this change introduces one, and Nightly will need either the LaunchDarkly default updated or an allowlist entry. --- _Generated by [Claude Code](https://claude.ai/code/session_015y1vTXBhMNwFSinPccnZCZ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Follow-up to #38067
…93) (#38049) Problem: mz_sinks, mz_kafka_sinks and mz_iceberg_sinks are builtin tables that pack_sink_update fills on the leader environmentd, on every catalog change. Getting rid of those writes is the point of this whole series ([SQL-118](https://linear.app/materializeinc/issue/SQL-118) / multi-envd), and the sink family is the last item in M1. Everything the three tables hold is already sitting in the persisted create_sql, so none of it has to be packed by Rust in the first place. Note mz_continual_tasks is named in the issue but no longer exists. The CT feature was removed in #35967, and the v81_to_v82 migration asserts no CT items remain, so there is nothing there to convert. The Linear description wants correcting. Solution: Two parts. parse_catalog_create_sql gets a CreateSink arm, alongside the CreateSource one it mirrors. It emits sink_type, cluster_id, connection_id, envelope_type, format/key_format/value_format, the kafka topic, and the iceberg namespace/table. The deprecated format collapse stays in Rust rather than moving into the view SQL, because its edges are fiddly and here they are unit-testable. Only avro/avro and json/json collapse to a single name, and a key format only exists once the sink declares a KEY, which is easy to get wrong reading the statement alone. Then all three tables become BuiltinMaterializedViews over mz_catalog_raw and pack_sink_update goes away, since packing them was its only job. Each view keeps its columns, order, nullability, keys, comments, access and ontology as they were, and reuses its old OID so pg identity does not move. No privileges column, unlike what mz_sources grew, to keep this a straight parity conversion. - three MigrationStep::replacement entries at the current dev version. - the old 0.160.0 step naming mz_sinks as a Table had to go with them: it no longer resolves, and validate_migration_steps panics on that at catalog open. Since that only reproduces against a catalog already on disk, there is now a unit test asserting every step resolves. Testing: - New test/sqllogictest/mz_sinks.slt pins the column shape of all three views, that they are materialized views, that non-sink items are excluded, and the JSON keys the view SQL expects back from the helper. Sinks cannot be created in sqllogictest at all, since purification always talks to the broker, so derived values are not covered here. - mz-sinks.td picks up what nothing asserted before: key_format on a keyed bare format, the composite key-text-value-text form, and mz_kafka_sinks.topic. - test/iceberg/mode-append.td picks up envelope_type = append and mz_iceberg_sinks.namespace/table. The iceberg path had no catalog coverage. - Unit tests in jsonb.rs for the new arm: bare and key/value formats with and without a KEY, all three envelope values, the avro/json collapse against text/text and text/bytes, a missing TOPIC and a missing TABLE, and a non-sink statement as a regression guard. - The tables.td "cannot insert into system table" assertions move off mz_kafka_sinks and onto mz_storage_usage_by_shard, which the migration validator refuses to convert. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.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 : )