fix: let read-only users query Recall on ClickHouse and read each table once (#1218)

* fix: read the Recall invocations view in one pass

The ClickHouse invocations view read recall_events three times and
joined four subqueries, so every lookup paid for three event reads and
hash-join preallocation. It is now one UNION ALL of the context rows,
heartbeats and lifecycle events folded by invocation, without joins.
Rows and column types are unchanged; the Recall schema version rises to
3 so writers replace the view through the existing upgrade path.

Refs: CHI-773

* fix: read recall_events once in the Recall statements view

The statements view read the events twice, once for statement facts
and once for resource names. It now folds one read by resource, which
names the resource and gathers its statement events, then by statement.
sql_available stays a lookup of the stored SQL, which is shared across
invocations.

Refs: CHI-773

* fix: let readonly = 1 users read Recall history on ClickHouse

A read connection sent SQLBuild's session settings and every recall
query set readonly = 2, which a readonly = 1 user may not do. A read
connection now applies the pinned session settings only when the
session may change settings; a query sets readonly = 2 only when the
session is not already read-only, sends the row limit only when the
user may set it and otherwise stops reading at the cap, and a setting
the user may not change fails with an error naming it and readonly = 2.
output_invocations is folded without joins and artifacts makes its
stored side nullable before the join, so no view depends on
join_use_nulls or sets SETTINGS.

Refs: CHI-772

* fix: read Recall tables without FINAL where folds ignore copies

Copies of a Recall key hold the same content, since a write with other
content is refused as a conflict. The invocations and statements views
only take the first, last, smallest or largest value of a key's copies,
so they read without FINAL, as do the delivery and context branches of
output_invocations. Views that list or count rows keep FINAL.

Refs: CHI-777

* fix: batch ClickHouse Recall writes over five seconds

A long command wrote about one small part per table every second.
ClickHouse batches now wait up to five seconds; heartbeats still go out
at once and a closing command sends everything it holds. A batch is
also sent early once a quarter of the queue's byte capacity is ready,
so a byte-heavy command fills the queue no sooner than before.

Refs: CHI-777

* docs: describe when a Recall reader skips session settings

Author: kvlonge

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`.