Verified Commit a741cace authored by Michael Usachenko's avatar Michael Usachenko Committed by GitLab
Browse files

chore(compiler): add query_cache_share_between_users setting

parent a998e1e6
Loading
Loading
Loading
Loading
+7 −0
Original line number Diff line number Diff line
@@ -950,6 +950,13 @@
          "format": "uint64",
          "minimum": 0
        },
        "query_cache_share_between_users": {
          "description": "ClickHouse `query_cache_share_between_users`. When true, cached\nresults are shared across ClickHouse users. Required in\nmulti-pod deployments where pods may connect as different users.",
          "type": [
            "boolean",
            "null"
          ]
        },
        "query_cache_ttl": {
          "description": "ClickHouse `query_cache_ttl` in seconds.",
          "type": [
+20 −2
Original line number Diff line number Diff line
@@ -38,6 +38,12 @@ pub struct QueryConfig {
    /// ClickHouse `query_cache_ttl` in seconds.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub query_cache_ttl: Option<u32>,

    /// ClickHouse `query_cache_share_between_users`. When true, cached
    /// results are shared across ClickHouse users. Required in
    /// multi-pod deployments where pods may connect as different users.
    #[serde(default, skip_serializing_if = "Option::is_none")]
    pub query_cache_share_between_users: Option<bool>,
}

impl QueryConfig {
@@ -48,6 +54,9 @@ impl QueryConfig {
            max_execution_time: overrides.max_execution_time.or(self.max_execution_time),
            use_query_cache: overrides.use_query_cache.or(self.use_query_cache),
            query_cache_ttl: overrides.query_cache_ttl.or(self.query_cache_ttl),
            query_cache_share_between_users: overrides
                .query_cache_share_between_users
                .or(self.query_cache_share_between_users),
        }
    }

@@ -162,16 +171,19 @@ mod tests {
            max_execution_time: Some(30),
            use_query_cache: Some(false),
            query_cache_ttl: Some(60),
            query_cache_share_between_users: Some(false),
        };
        let over = QueryConfig {
            max_execution_time: Some(120),
            use_query_cache: None,
            query_cache_ttl: None,
            query_cache_share_between_users: Some(true),
        };
        let merged = base.merge(&over);
        assert_eq!(merged.max_execution_time, Some(120));
        assert_eq!(merged.use_query_cache, Some(false));
        assert_eq!(merged.query_cache_ttl, Some(60));
        assert_eq!(merged.query_cache_share_between_users, Some(true));
    }

    #[test]
@@ -180,12 +192,17 @@ mod tests {
            max_execution_time: Some(30),
            use_query_cache: Some(true),
            query_cache_ttl: None,
            query_cache_share_between_users: Some(true),
        };
        let mut settings = cfg.to_clickhouse_settings()?;
        settings.sort_by(|a, b| a.0.cmp(&b.0));
        assert_eq!(settings.len(), 2);
        assert_eq!(settings.len(), 3);
        assert_eq!(settings[0], ("max_execution_time".into(), "30".into()));
        assert_eq!(settings[1], ("use_query_cache".into(), "1".into()));
        assert_eq!(
            settings[1],
            ("query_cache_share_between_users".into(), "1".into())
        );
        assert_eq!(settings[2], ("use_query_cache".into(), "1".into()));
        Ok(())
    }

@@ -211,6 +228,7 @@ mod tests {
                max_execution_time: Some(30),
                use_query_cache: Some(false),
                query_cache_ttl: Some(60),
                query_cache_share_between_users: None,
            },
            overrides,
        };
+8 −0
Original line number Diff line number Diff line
@@ -489,6 +489,14 @@ fn cursor_pagination_validation() {
        "cursor query should enable CH query cache: {}",
        result.base.sql
    );
    assert!(
        result
            .base
            .sql
            .contains("query_cache_share_between_users = 1"),
        "cursor query should share cache between users: {}",
        result.base.sql
    );

    // offset + page_size > limit rejected
    let err = compile(
+6 −0
Original line number Diff line number Diff line
@@ -880,6 +880,7 @@ mod tests {
            max_execution_time: None,
            use_query_cache: Some(true),
            query_cache_ttl: Some(60),
            query_cache_share_between_users: Some(true),
        };
        let result = codegen(&Node::Query(Box::new(q)), empty_ctx(), cfg).unwrap();
        assert!(
@@ -888,6 +889,11 @@ mod tests {
            result.sql,
        );
        assert!(result.sql.contains("use_query_cache = 1"), "{}", result.sql);
        assert!(
            result.sql.contains("query_cache_share_between_users = 1"),
            "{}",
            result.sql
        );
        assert!(
            result.sql.contains("query_cache_ttl = 60"),
            "{}",
+29 −0
Original line number Diff line number Diff line
@@ -16,6 +16,12 @@ pub fn resolve(query_type: &str, has_cursor: bool) -> QueryConfig {
        // cache so that subsequent pages hit the same cached result. This
        // overrides even an explicit `use_query_cache: false` in YAML.
        cfg.use_query_cache = Some(true);
        // Default to sharing cache across ClickHouse users so all pods
        // hit the same cached result. Respects an explicit operator
        // override (e.g. `query_cache_share_between_users: false` in YAML).
        if cfg.query_cache_share_between_users.is_none() {
            cfg.query_cache_share_between_users = Some(true);
        }
    }

    cfg
@@ -35,6 +41,7 @@ mod tests {
    fn resolve_enables_cache_for_cursor() {
        let cfg = resolve("search", true);
        assert_eq!(cfg.use_query_cache, Some(true));
        assert_eq!(cfg.query_cache_share_between_users, Some(true));
    }

    #[test]
@@ -43,4 +50,26 @@ mod tests {
        // Without global init, default is None
        assert_eq!(cfg.use_query_cache, None);
    }

    #[test]
    fn resolve_respects_explicit_share_between_users_false() {
        // When an operator explicitly sets query_cache_share_between_users: false
        // in YAML, cursor pagination should not override it.
        // Without global init the field defaults to None, so we test the
        // code path directly: if the field is already Some(false), the
        // is_none() guard should skip it.
        let mut cfg = resolve("search", true);
        // Simulate an operator having set it to false before resolve ran.
        // (In practice this comes from the YAML config merge.)
        cfg.query_cache_share_between_users = Some(false);
        // Re-apply the cursor logic manually to verify the guard.
        if cfg.query_cache_share_between_users.is_none() {
            cfg.query_cache_share_between_users = Some(true);
        }
        assert_eq!(
            cfg.query_cache_share_between_users,
            Some(false),
            "explicit false should not be overridden"
        );
    }
}