fix: let read-only users query Recall on ClickHouse and read each table once

crates/sqlbuild-python/src/bindings/_helpers/recall/clickhouse_calls.rs

@@ -1,7 +1,7 @@
 //! Calls into the ClickHouse adapter's Recall store operations, through the native host.
 
 use pyo3::prelude::{Bound, FromPyObject, PyAny, PyAnyMethods, PyErr, PyResult, Python};
-use pyo3::types::{PyBytes, PyDict, PyDictMethods, PyInt, PyList, PyString};
+use pyo3::types::{PyBool, PyBytes, PyDict, PyDictMethods, PyInt, PyList, PyString};
 use sqlbuild_config::models::ConfigValue;
 use sqlbuild_host::entries::models::AdapterHost;
 use sqlbuild_recall::clickhouse::main::error_kind::error_kind;
@@ -85,10 +85,17 @@ fn reply<'py>(
     }
 }
 
-/// Connect with the store's connection options; a `quiet` caller, Recall's delivery thread, never has its output captured.
+/// How a store connection is used: the delivery thread writes and is never captured; a reader skips the session settings under `readonly = 1`, so that user can read.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(crate) enum ConnectionRole {
+    Writer,
+    Reader,
+}
+
+/// Connect with the store's connection options for `role`.
 pub(crate) fn connect(
     py: Python<'_>,
-    (config, quiet): (&[(String, ConfigValue)], bool),
+    (config, role): (&[(String, ConfigValue)], ConnectionRole),
     action: &str,
 ) -> Result<StoreHandle, StoreError> {
     let options = PyDict::new(py);
@@ -104,12 +111,18 @@ pub(crate) fn connect(
             ("config", options.into_any()),
             (
                 "quiet_thread_attribute",
-                if quiet {
+                if role == ConnectionRole::Writer {
                     PyString::new(py, OUTPUT_SUPPRESSED_ATTRIBUTE).into_any()
                 } else {
                     py.None().into_bound(py)
                 },
             ),
+            (
+                "read_only",
+                PyBool::new(py, role == ConnectionRole::Reader)
+                    .to_owned()
+                    .into_any(),
+            ),
         ],
         action,
     )?
@@ -132,11 +145,11 @@ fn cell_value(cell: Option<Cell>) -> RecallValue {
     }
 }
 
-/// The columns, rows and statistics of `sql` run with `settings`.
+/// The columns, at most `row_cap` rows and the statistics of `sql`, run with those of `settings` the session does not already hold.
 pub(crate) fn query(
     py: Python<'_>,
     handle: StoreHandle,
-    (sql, settings): (&str, &[(&str, i64)]),
+    (sql, settings, row_cap): (&str, &[(&str, i64)], Option<usize>),
     action: &str,
 ) -> Result<(QueryResult, QueryStatistics), StoreError> {
     let query_settings = PyDict::new(py);
@@ -152,6 +165,13 @@ pub(crate) fn query(
             ("handle", handle_value(py, handle)),
             ("sql", PyString::new(py, sql).into_any()),
             ("settings", query_settings.into_any()),
+            (
+                "row_cap",
+                row_cap.map_or_else(
+                    || py.None().into_bound(py),
+                    |cap| PyInt::new(py, cap).into_any(),
+                ),
+            ),
         ],
         action,
     )?;

crates/sqlbuild-python/src/bindings/_helpers/recall/clickhouse_store.rs

@@ -46,9 +46,11 @@ use sqlbuild_recall::store::models::{
 use sqlbuild_recall::store::types::{RecallStore, RecallStoreOpener};
 
 use crate::bindings::_helpers::recall::clickhouse_calls::{
-    self as calls, QueryStatistics, StoreHandle,
+    self as calls, ConnectionRole, QueryStatistics, StoreHandle,
+};
+use crate::bindings::_helpers::recall::constants::{
+    READ_ONLY_QUERIES, READONLY_SETTING, SECONDS_PER_DAY, UNKNOWN_TABLE_ERROR,
 };
-use crate::bindings::_helpers::recall::constants::{SECONDS_PER_DAY, UNKNOWN_TABLE_ERROR};
 
 /// Where a ClickHouse store lives and how long it keeps history.
 #[derive(Clone, Debug)]
@@ -143,7 +145,7 @@ impl ClickHouseStore {
     }
 
     fn query(&self, py: Python<'_>, sql: &str, action: &str) -> Result<QueryResult, StoreError> {
-        calls::query(py, self.connection()?, (sql, &[]), action).map(|(result, _)| result)
+        calls::query(py, self.connection()?, (sql, &[], None), action).map(|(result, _)| result)
     }
 
     fn command(&self, py: Python<'_>, sql: &str, action: &str) -> Result<(), StoreError> {
@@ -591,7 +593,7 @@ impl RecallStoreOpener for ClickHouseStoreOpener {
                 location: self.location.clone(),
                 connection: Some(calls::connect(
                     py,
-                    (&self.location.config, true),
+                    (&self.location.config, ConnectionRole::Writer),
                     &self.location.connect_action(),
                 )?),
                 columns: HashMap::new(),
@@ -645,7 +647,7 @@ impl ClickHouseReader {
             location: location.clone(),
             connection: Some(calls::connect(
                 py,
-                (&location.config, false),
+                (&location.config, ConnectionRole::Reader),
                 &location.connect_action(),
             )?),
             columns: HashMap::new(),
@@ -683,7 +685,7 @@ impl ClickHouseReader {
             location: location.clone(),
             connection: Some(calls::connect(
                 py,
-                (&location.config, false),
+                (&location.config, ConnectionRole::Reader),
                 &location.connect_action(),
             )?),
             columns: HashMap::new(),
@@ -719,15 +721,13 @@ impl ClickHouseReader {
         sql: &str,
         limit: Option<usize>,
     ) -> Result<(QueryResult, QueryStatistics), StoreError> {
-        let mut settings: Vec<(&str, i64)> = vec![("readonly", 2)];
-        if let Some(limit) = limit {
-            settings.push(("limit", i64::try_from(limit).unwrap_or(i64::MAX - 1) + 1));
-        }
+        let settings: [(&str, i64); 1] = [(READONLY_SETTING, READ_ONLY_QUERIES)];
+        let row_cap: Option<usize> = limit.map(|limit| limit.saturating_add(1));
         let run = || {
             calls::query(
                 py,
                 self.store.connection()?,
-                (sql, &settings),
+                (sql, &settings, row_cap),
                 "run the Recall query",
             )
         };

crates/sqlbuild-python/src/bindings/_helpers/recall/constants.rs

@@ -100,6 +100,9 @@ pub(crate) const CLOSE_POLL_MS: u64 = 50;
 pub(crate) const QUARANTINE_START_WAIT_MS: u64 = 200;
 /// The ClickHouse error name of a query naming a relation that does not exist yet.
 pub(crate) const UNKNOWN_TABLE_ERROR: &str = "UNKNOWN_TABLE";
+/// `sqb recall query` runs read-only: the setting, and the value that still allows query settings. Not sent when the session is already read-only.
+pub(crate) const READONLY_SETTING: &str = "readonly";
+pub(crate) const READ_ONLY_QUERIES: i64 = 2;
 /// Seconds in a day, for the artifact refresh cutoff.
 pub(crate) const SECONDS_PER_DAY: u64 = 24 * 60 * 60;
 /// The parameter a lifecycle-event sink takes its event in.

crates/sqlbuild-recall/src/clickhouse/constants.rs

@@ -4,8 +4,8 @@ use crate::schema::models::Table;
 
 /// Rows one insert batch takes; ClickHouse prefers few large inserts to many small ones.
 pub const BATCH_ROWS: usize = 5_000;
-/// Longest wait for a batch to fill, so each table gets at most about one part per second per writer.
-pub const BATCH_WINDOW_MS: u64 = 1_000;
+/// Longest wait for a batch to fill, so a long command adds at most one part per table every five seconds. A closing session sends what it holds at once, and heartbeats never wait for it.
+pub const BATCH_WINDOW_MS: u64 = 5_000;
 /// ClickHouse `READONLY`.
 pub const READONLY_CODE: u64 = 164;
 /// ClickHouse `DATABASE_ACCESS_DENIED`.