diff --git a/Cargo.toml b/Cargo.toml index 9cdb79f5e2d16..e0fd70a20f5b3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -399,4 +399,5 @@ disallowed_types = "warn" from_over_into = "warn" mod_module_files = "warn" needless_pass_by_ref_mut = "warn" +borrow_interior_mutable_const = "warn" # END LINT CONFIG diff --git a/misc/python/materialize/cli/gen-lints.py b/misc/python/materialize/cli/gen-lints.py index 2e808fa46efce..2e073e8bda421 100644 --- a/misc/python/materialize/cli/gen-lints.py +++ b/misc/python/materialize/cli/gen-lints.py @@ -175,6 +175,9 @@ # We consistently don't use `mod.rs` files. "mod_module_files", "needless_pass_by_ref_mut", + # Helps prevent bugs caused by wrong usage of `const` instead of `static` + # to define global mutable values. + "borrow_interior_mutable_const", ] MESSAGE_LINT_MISSING = ( diff --git a/misc/wasm/Cargo.toml b/misc/wasm/Cargo.toml index ce33ec7ae1b50..e0c0ab40abab5 100644 --- a/misc/wasm/Cargo.toml +++ b/misc/wasm/Cargo.toml @@ -83,4 +83,5 @@ disallowed_types = "warn" from_over_into = "warn" mod_module_files = "warn" needless_pass_by_ref_mut = "warn" +borrow_interior_mutable_const = "warn" # END LINT CONFIG diff --git a/src/adapter/src/coord/command_handler.rs b/src/adapter/src/coord/command_handler.rs index b9cb45ca2e1d1..8667f1af994aa 100644 --- a/src/adapter/src/coord/command_handler.rs +++ b/src/adapter/src/coord/command_handler.rs @@ -1098,7 +1098,7 @@ impl Coordinator { /// /// (Note that the chosen timestamp won't be the same timestamp as the system table inserts, /// unfortunately.) - async fn resolve_mz_now_for_create_materialized_view<'a>( + async fn resolve_mz_now_for_create_materialized_view( &mut self, cmvs: &CreateMaterializedViewStatement, resolved_ids: &ResolvedIds, diff --git a/src/adapter/src/coord/timeline.rs b/src/adapter/src/coord/timeline.rs index a968651d7e626..da0b843a88292 100644 --- a/src/adapter/src/coord/timeline.rs +++ b/src/adapter/src/coord/timeline.rs @@ -12,8 +12,6 @@ use std::collections::{BTreeMap, BTreeSet}; use std::fmt; use std::sync::Arc; -use std::sync::LazyLock; -use std::time::Duration; use chrono::{DateTime, Utc}; use futures::Future; @@ -711,16 +709,8 @@ impl Coordinator { /// Convenience function for calculating the current upper bound that we want to /// prevent the global timestamp from exceeding. fn upper_bound(now: &mz_repr::Timestamp) -> mz_repr::Timestamp { - const TIMESTAMP_INTERVAL: LazyLock = LazyLock::new(|| { - Duration::from_secs(5) - .as_millis() - .try_into() - .expect("5 seconds can fit into `Timestamp`") - }); - + const TIMESTAMP_INTERVAL_MS: u64 = 5000; const TIMESTAMP_INTERVAL_UPPER_BOUND: u64 = 2; - now.saturating_add( - TIMESTAMP_INTERVAL.saturating_mul(Timestamp::from(TIMESTAMP_INTERVAL_UPPER_BOUND)), - ) + now.saturating_add(TIMESTAMP_INTERVAL_MS * TIMESTAMP_INTERVAL_UPPER_BOUND) } diff --git a/src/adapter/src/optimize.rs b/src/adapter/src/optimize.rs index d93595b5ab739..05ef40d580657 100644 --- a/src/adapter/src/optimize.rs +++ b/src/adapter/src/optimize.rs @@ -123,18 +123,13 @@ where #[mz_ore::instrument(target = "optimizer", level = "debug", name = "optimize")] fn catch_unwind_optimize(&mut self, plan: From) -> Result { match mz_ore::panic::catch_unwind_str(AssertUnwindSafe(|| self.optimize(plan))) { - Ok(result) => { - match result.map_err(Into::into) { - Err(OptimizerError::TransformError(TransformError::CallerShouldPanic(msg))) => { - // Promote a `CallerShouldPanic` error from the result - // to a proper panic. This is needed in order to ensure - // that `mz_unsafe.mz_panic('forced panic')` calls still - // panic the caller. - panic!("{}", msg) - } - result => result, - } + Ok(Err(OptimizerError::TransformError(TransformError::CallerShouldPanic(msg)))) => { + // Promote a `CallerShouldPanic` error from the result to a proper panic. This is + // needed in order to ensure that `mz_unsafe.mz_panic('forced panic')` calls still + // panic the caller. + panic!("{msg}"); } + Ok(result) => result, Err(panic) => { let msg = format!("unexpected panic during query optimization: {panic}"); Err(OptimizerError::Internal(msg)) diff --git a/src/adapter/src/session.rs b/src/adapter/src/session.rs index 7f5ae73ac6809..3315fb3f4a5aa 100644 --- a/src/adapter/src/session.rs +++ b/src/adapter/src/session.rs @@ -715,7 +715,7 @@ impl Session { datums: Row::pack(params.iter().map(|(d, _t)| d)), types: params.into_iter().map(|(_d, t)| t).collect(), }, - result_formats: result_formats.into_iter().map(Into::into).collect(), + result_formats, state: PortalState::NotStarted, logging, }, diff --git a/src/catalog/src/durable/initialize.rs b/src/catalog/src/durable/initialize.rs index 27f100d077245..713a4b24c2dd3 100644 --- a/src/catalog/src/durable/initialize.rs +++ b/src/catalog/src/durable/initialize.rs @@ -114,21 +114,17 @@ const DEFAULT_ALLOCATOR_ID: u64 = 1; pub const DEFAULT_USER_NETWORK_POLICY_ID: NetworkPolicyId = NetworkPolicyId::User(1); pub const DEFAULT_USER_NETWORK_POLICY_NAME: &str = "default"; -pub const DEFAULT_USER_NETWORK_POLICY_RULES: LazyLock< - Vec<( - &str, - mz_sql::plan::NetworkPolicyRuleAction, - mz_sql::plan::NetworkPolicyRuleDirection, - &str, - )>, -> = LazyLock::new(|| { - vec![( - "open_ingress", - mz_sql::plan::NetworkPolicyRuleAction::Allow, - mz_sql::plan::NetworkPolicyRuleDirection::Ingress, - "0.0.0.0/0", - )] -}); +pub const DEFAULT_USER_NETWORK_POLICY_RULES: &[( + &str, + mz_sql::plan::NetworkPolicyRuleAction, + mz_sql::plan::NetworkPolicyRuleDirection, + &str, +)] = &[( + "open_ingress", + mz_sql::plan::NetworkPolicyRuleAction::Allow, + mz_sql::plan::NetworkPolicyRuleDirection::Ingress, + "0.0.0.0/0", +)]; static DEFAULT_USER_NETWORK_POLICY_PRIVILEGES: LazyLock> = LazyLock::new(|| { vec![rbac::owner_privilege( @@ -594,7 +590,6 @@ pub(crate) async fn initialize( DEFAULT_USER_NETWORK_POLICY_ID, DEFAULT_USER_NETWORK_POLICY_NAME.to_string(), DEFAULT_USER_NETWORK_POLICY_RULES - .clone() .into_iter() .map(|(name, action, direction, ip_str)| NetworkPolicyRule { name: name.to_string(), diff --git a/src/catalog/tests/debug.rs b/src/catalog/tests/debug.rs index 80f13f6ac8b29..1aabbd8548cb7 100644 --- a/src/catalog/tests/debug.rs +++ b/src/catalog/tests/debug.rs @@ -155,7 +155,7 @@ async fn test_persist_debug() { test_debug(state_builder).await; } -async fn test_debug<'a>(state_builder: TestCatalogStateBuilder) { +async fn test_debug(state_builder: TestCatalogStateBuilder) { let state_builder = state_builder.with_default_deploy_generation(); let mut openable_state1 = state_builder.clone().unwrap_build().await; // Check initial empty trace. @@ -340,7 +340,7 @@ async fn test_persist_debug_edit_fencing() { test_debug_edit_fencing(state_builder).await; } -async fn test_debug_edit_fencing<'a>(state_builder: TestCatalogStateBuilder) { +async fn test_debug_edit_fencing(state_builder: TestCatalogStateBuilder) { let mut state = state_builder .clone() .with_default_deploy_generation() @@ -435,7 +435,7 @@ async fn test_persist_debug_delete_fencing() { test_debug_delete_fencing(state_builder).await; } -async fn test_debug_delete_fencing<'a>(state_builder: TestCatalogStateBuilder) { +async fn test_debug_delete_fencing(state_builder: TestCatalogStateBuilder) { let mut state = state_builder .clone() .with_default_deploy_generation() diff --git a/src/expr-test-util/src/lib.rs b/src/expr-test-util/src/lib.rs index ae89d4cd599d6..e12e3fa2dafd7 100644 --- a/src/expr-test-util/src/lib.rs +++ b/src/expr-test-util/src/lib.rs @@ -239,7 +239,7 @@ impl MirScalarExprDeserializeContext { I: Iterator, { match &first_arg { - TokenTree::Ident(i) if i.to_string().to_ascii_lowercase() == "ok" => { + TokenTree::Ident(i) if i.to_string().eq_ignore_ascii_case("ok") => { // literal definition is mandatory after OK token let first_arg = if let Some(first_arg) = rest_of_stream.next() { first_arg @@ -251,7 +251,7 @@ impl MirScalarExprDeserializeContext { _ => Err(format!("expected literal after Ident: `{}`", i)), } } - TokenTree::Ident(i) if i.to_string().to_ascii_lowercase() == "err" => { + TokenTree::Ident(i) if i.to_string().eq_ignore_ascii_case("err") => { let error = deserialize_generic(rest_of_stream, "EvalError")?; let typ: Option = deserialize_optional_generic(rest_of_stream, "ScalarType")?; diff --git a/src/frontegg-mock/src/handlers/user.rs b/src/frontegg-mock/src/handlers/user.rs index 436bec5495fa9..f32ae97e2fdd1 100644 --- a/src/frontegg-mock/src/handlers/user.rs +++ b/src/frontegg-mock/src/handlers/user.rs @@ -27,7 +27,7 @@ use std::sync::Arc; use uuid::Uuid; // https://docs.frontegg.com/reference/userscontrollerv2_getuserprofile -pub async fn handle_get_user_profile<'a>( +pub async fn handle_get_user_profile( State(context): State>, TypedHeader(authorization): TypedHeader>, ) -> Result, StatusCode> { @@ -41,7 +41,7 @@ pub async fn handle_get_user_profile<'a>( } // https://docs.frontegg.com/reference/userapitokensv1controller_createtenantapitoken -pub async fn handle_post_user_api_token<'a>( +pub async fn handle_post_user_api_token( State(context): State>, TypedHeader(authorization): TypedHeader>, Json(request): Json, diff --git a/src/lsp-server/tests/test.rs b/src/lsp-server/tests/test.rs index 7a8aa24436389..2068fc8fc8b25 100644 --- a/src/lsp-server/tests/test.rs +++ b/src/lsp-server/tests/test.rs @@ -51,9 +51,9 @@ mod tests { } /// The file path used during the tests is where the SQL code resides. - const FILE_PATH: LazyLock = LazyLock::new(|| temp_dir().join("foo.sql")); + static FILE_PATH: LazyLock = LazyLock::new(|| temp_dir().join("foo.sql")); /// The SQL code written inside [FILE_PATH]. - const FILE_SQL_CONTENT: LazyLock = + static FILE_SQL_CONTENT: LazyLock = LazyLock::new(|| "SELECT \t\t\t200, 200;".to_string()); /// Tests the different capabilities of [Backend](mz_lsp::backend::Backend) diff --git a/src/mz/src/config_file.rs b/src/mz/src/config_file.rs index dad432239945f..a86b04467ebed 100644 --- a/src/mz/src/config_file.rs +++ b/src/mz/src/config_file.rs @@ -198,7 +198,7 @@ impl ConfigFile { } /// Removes a profile from the configuration file. - pub async fn remove_profile<'a>(&self, name: &str) -> Result<(), Error> { + pub async fn remove_profile(&self, name: &str) -> Result<(), Error> { let mut editable = self.editable.clone(); let profiles = editable["profiles"] .as_table_mut() diff --git a/src/persist-client/src/rpc.rs b/src/persist-client/src/rpc.rs index 9d06e4b01382a..fc3a086bc1d4d 100644 --- a/src/persist-client/src/rpc.rs +++ b/src/persist-client/src/rpc.rs @@ -1135,9 +1135,9 @@ mod pubsub_state { use crate::rpc::{PubSubSenderInternal, PubSubState}; use crate::ShardId; - const SHARD_ID_0: LazyLock = + static SHARD_ID_0: LazyLock = LazyLock::new(|| ShardId::from_str("s00000000-0000-0000-0000-000000000000").unwrap()); - const SHARD_ID_1: LazyLock = + static SHARD_ID_1: LazyLock = LazyLock::new(|| ShardId::from_str("s11111111-1111-1111-1111-111111111111").unwrap()); const VERSIONED_DATA_0: VersionedData = VersionedData { diff --git a/src/prof-http/src/lib.rs b/src/prof-http/src/lib.rs index 5b389f63d2277..ca3aa3f913f0d 100644 --- a/src/prof-http/src/lib.rs +++ b/src/prof-http/src/lib.rs @@ -87,7 +87,7 @@ pub struct FlamegraphTemplate<'a> { } #[allow(dropping_copy_types)] -async fn time_prof<'a>( +async fn time_prof( merge_threads: bool, build_info: &BuildInfo, // the time in seconds to run the profiler for diff --git a/src/secrets/src/cache.rs b/src/secrets/src/cache.rs index 09c635941589d..985f59e5cb9fd 100644 --- a/src/secrets/src/cache.rs +++ b/src/secrets/src/cache.rs @@ -19,7 +19,7 @@ use mz_repr::CatalogItemId; use crate::{CachingPolicy, SecretsReader}; /// Default "time to live" for a single cache value, represented in __seconds__. -pub const DEFAULT_TTL_SECS: AtomicU64 = AtomicU64::new(Duration::from_secs(300).as_secs()); +pub const DEFAULT_TTL_SECS: u64 = Duration::from_secs(300).as_secs(); #[derive(Debug)] struct CachingParameters { @@ -53,7 +53,7 @@ impl Default for CachingParameters { fn default() -> Self { CachingParameters { enabled: AtomicBool::new(true), - ttl_secs: DEFAULT_TTL_SECS, + ttl_secs: AtomicU64::new(DEFAULT_TTL_SECS), } } } diff --git a/src/sql/src/plan/statement/ddl.rs b/src/sql/src/plan/statement/ddl.rs index 1589e7040d33e..8af4d53ad0c93 100644 --- a/src/sql/src/plan/statement/ddl.rs +++ b/src/sql/src/plan/statement/ddl.rs @@ -172,7 +172,7 @@ mod connection; // more strict. const MAX_NUM_COLUMNS: usize = 256; -const MANAGED_REPLICA_PATTERN: std::sync::LazyLock = +static MANAGED_REPLICA_PATTERN: std::sync::LazyLock = std::sync::LazyLock::new(|| regex::Regex::new(r"^r(\d)+$").unwrap()); /// Given a relation desc and a column list, checks that: diff --git a/src/sql/src/session/vars.rs b/src/sql/src/session/vars.rs index ba98083f18eb1..f851f03f3e5e5 100644 --- a/src/sql/src/session/vars.rs +++ b/src/sql/src/session/vars.rs @@ -419,7 +419,7 @@ impl SessionVars { &WELCOME_MESSAGE, ] .into_iter() - .chain(SystemVars::SESSION_VARS.iter().map(|(_name, var)| *var)) + .chain(SESSION_SYSTEM_VARS.iter().map(|(_name, var)| *var)) .map(|var| (var.name, SessionVar::new(var.clone()))) .collect(); @@ -1070,37 +1070,6 @@ impl Default for SystemVars { } impl SystemVars { - /// Set of [`SystemVar`]s that can also get set at a per-Session level. - /// - /// TODO(parkmycar): Instead of a separate list, make this a field on VarDefinition. - const SESSION_VARS: LazyLock> = - LazyLock::new(|| { - [ - &APPLICATION_NAME, - &CLIENT_ENCODING, - &CLIENT_MIN_MESSAGES, - &CLUSTER, - &CLUSTER_REPLICA, - &CURRENT_OBJECT_MISSING_WARNINGS, - &DATABASE, - &DATE_STYLE, - &EXTRA_FLOAT_DIGITS, - &INTEGER_DATETIMES, - &INTERVAL_STYLE, - &REAL_TIME_RECENCY_TIMEOUT, - &SEARCH_PATH, - &STANDARD_CONFORMING_STRINGS, - &STATEMENT_TIMEOUT, - &IDLE_IN_TRANSACTION_SESSION_TIMEOUT, - &TIMEZONE, - &TRANSACTION_ISOLATION, - &MAX_QUERY_RESULT_SIZE, - ] - .into_iter() - .map(|var| (UncasedStr::new(var.name()), var)) - .collect() - }); - pub fn new() -> Self { let system_vars = vec![ &MAX_KAFKA_CONNECTIONS, @@ -1273,7 +1242,7 @@ impl SystemVars { // Include all of our feature flags. .chain(definitions::FEATURE_FLAGS.iter().copied()) // Include the subset of Session variables we allow system defaults for. - .chain(Self::SESSION_VARS.values().copied()) + .chain(SESSION_SYSTEM_VARS.values().copied()) .cloned() // Include Persist configs. .chain(dyncfg_vars) @@ -1341,7 +1310,7 @@ impl SystemVars { self.vars .values() .map(|v| v.as_var()) - .filter(|v| !Self::SESSION_VARS.contains_key(UncasedStr::new(v.name()))) + .filter(|v| !SESSION_SYSTEM_VARS.contains_key(UncasedStr::new(v.name()))) } /// Returns an iterator over the configuration parameters and their current @@ -1356,12 +1325,12 @@ impl SystemVars { self.vars .values() .map(|v| v.as_var()) - .filter(|v| Self::SESSION_VARS.contains_key(UncasedStr::new(v.name()))) + .filter(|v| SESSION_SYSTEM_VARS.contains_key(UncasedStr::new(v.name()))) } /// Returns whether or not this parameter can be modified by a superuser. pub fn user_modifiable(&self, name: &str) -> bool { - Self::SESSION_VARS.contains_key(UncasedStr::new(name)) + SESSION_SYSTEM_VARS.contains_key(UncasedStr::new(name)) || name == ENABLE_RBAC_CHECKS.name() || name == NETWORK_POLICY.name() } @@ -2332,6 +2301,37 @@ pub fn is_http_config_var(name: &str) -> bool { name == WEBHOOK_CONCURRENT_REQUEST_LIMIT.name() } +/// Set of [`SystemVar`]s that can also get set at a per-Session level. +/// +/// TODO(parkmycar): Instead of a separate list, make this a field on VarDefinition. +static SESSION_SYSTEM_VARS: LazyLock> = + LazyLock::new(|| { + [ + &APPLICATION_NAME, + &CLIENT_ENCODING, + &CLIENT_MIN_MESSAGES, + &CLUSTER, + &CLUSTER_REPLICA, + &CURRENT_OBJECT_MISSING_WARNINGS, + &DATABASE, + &DATE_STYLE, + &EXTRA_FLOAT_DIGITS, + &INTEGER_DATETIMES, + &INTERVAL_STYLE, + &REAL_TIME_RECENCY_TIMEOUT, + &SEARCH_PATH, + &STANDARD_CONFORMING_STRINGS, + &STATEMENT_TIMEOUT, + &IDLE_IN_TRANSACTION_SESSION_TIMEOUT, + &TIMEZONE, + &TRANSACTION_ISOLATION, + &MAX_QUERY_RESULT_SIZE, + ] + .into_iter() + .map(|var| (UncasedStr::new(var.name()), var)) + .collect() + }); + // Provides a wrapper to express that a particular `ServerVar` is meant to be used as a feature /// flag. #[derive(Debug)] diff --git a/src/sql/src/session/vars/definitions.rs b/src/sql/src/session/vars/definitions.rs index 55e7762c48a3a..8cd76d329aa9b 100644 --- a/src/sql/src/session/vars/definitions.rs +++ b/src/sql/src/session/vars/definitions.rs @@ -914,9 +914,7 @@ pub static SENTRY_FILTERS: VarDefinition = VarDefinition::new_lazy( pub static WEBHOOKS_SECRETS_CACHING_TTL_SECS: VarDefinition = VarDefinition::new_lazy( "webhooks_secrets_caching_ttl_secs", lazy_value!(usize; || { - usize::cast_from( - mz_secrets::cache::DEFAULT_TTL_SECS.load(std::sync::atomic::Ordering::Relaxed), - ) + usize::cast_from(mz_secrets::cache::DEFAULT_TTL_SECS) }), "Sets the time-to-live for values in the Webhooks secrets cache.", false, diff --git a/src/storage/src/source/mysql/replication.rs b/src/storage/src/source/mysql/replication.rs index d947f71d14e8c..9594611073c59 100644 --- a/src/storage/src/source/mysql/replication.rs +++ b/src/storage/src/source/mysql/replication.rs @@ -484,7 +484,7 @@ pub(crate) fn render>( /// Produces the replication stream from the MySQL server. This will return all transactions /// whose GTIDs were not present in the GTID UUIDs referenced in the `resume_uppper` partitions. -async fn raw_stream<'a>( +async fn raw_stream( config: &RawSourceCreationConfig, mut conn: MySqlConn, resume_upper: &Antichain, diff --git a/src/storage/src/source/mysql/snapshot.rs b/src/storage/src/source/mysql/snapshot.rs index 83c13076543d1..a8921eed097bc 100644 --- a/src/storage/src/source/mysql/snapshot.rs +++ b/src/storage/src/source/mysql/snapshot.rs @@ -502,7 +502,7 @@ pub(crate) fn render>( } /// Fetch the size of the snapshot on this worker. -async fn fetch_snapshot_size<'a, Q>( +async fn fetch_snapshot_size( conn: &mut Q, tables: Vec<(MySqlTableName, usize)>, metrics: MySqlSnapshotMetrics,