fix: report misconfigured Recall write connections and skip unseen views

An unknown write connection, a non-string adapter, a SQLite connection without a path, an empty ClickHouse database or an unterminated environment reference now fails the command with C975, like a write connection without a read connection; an unset environment variable still only skips recording. A ClickHouse writer that sees none of the views, as when granted only the tables, issues no view DDL, warns of no view upgrade and records no version it cannot vouch for. A test checks that both dialects' views expose the same columns.

Author: kvlonge

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

@@ -195,6 +195,7 @@ impl ClickHouseStore {
             return Ok(StoreVersion::Recorded {
                 version: state.version,
                 age_seconds: state.version_age_seconds,
+                views_visible: !state.views.is_empty(),
             });
         }
         let mut expected = view_columns().map_err(|message| StoreError {
@@ -225,7 +226,11 @@ impl ClickHouseStore {
                 .or_default()
                 .insert(text(row.get(1)));
         }
-        Ok(StoreVersion::Unrecorded(schema_age(&expected, &observed)))
+        Ok(StoreVersion::Unrecorded(schema_age(
+            &expected,
+            &observed,
+            &VIEW_NAMES,
+        )))
     }
 
     fn require_database(&self, py: Python<'_>) -> Result<(), StoreError> {
@@ -325,7 +330,12 @@ impl ClickHouseStore {
                 tables,
             });
         }
-        for (name, statement) in view_statements(Dialect::ClickHouse, &self.location.options) {
+        let view_ddl: Vec<(&str, String)> = if plan.view_ddl {
+            view_statements(Dialect::ClickHouse, &self.location.options)
+        } else {
+            Vec::new()
+        };
+        for (name, statement) in view_ddl {
             let statement: String = if plan.replace_views {
                 statement
             } else if !views.contains(name) {
@@ -920,6 +930,8 @@ fn observed_version(store: StoreVersion) -> i64 {
     match store {
         StoreVersion::Recorded { version, .. } => version,
         StoreVersion::Unrecorded(SchemaAge::Older) => 0,
-        StoreVersion::Unrecorded(SchemaAge::Current | SchemaAge::Newer) => RECALL_SCHEMA_VERSION,
+        StoreVersion::Unrecorded(SchemaAge::Current | SchemaAge::Newer | SchemaAge::Unseen) => {
+            RECALL_SCHEMA_VERSION
+        }
     }
 }

crates/sqlbuild-recall/src/schema/main/schema_age.rs

@@ -4,12 +4,16 @@ use std::collections::{BTreeMap, BTreeSet};
 
 use crate::schema::models::SchemaAge;
 
-/// Compare the `observed` columns of each table and view with this build's `expected` ones; both evolve additively, so a missing relation or column means an older store and an unknown column a newer one. A relation this build does not define is ignored.
+/// Compare the `observed` columns of each table and view with this build's `expected` ones; both evolve additively, so a missing relation or column means an older store and an unknown column a newer one. When none of `views` is observed, the user may simply not see them, so nothing is judged. A relation this build does not define is ignored.
 #[must_use]
 pub fn schema_age(
     expected: &BTreeMap<&'static str, BTreeSet<String>>,
     observed: &BTreeMap<String, BTreeSet<String>>,
+    views: &[&str],
 ) -> SchemaAge {
+    if !views.iter().any(|view| observed.contains_key(*view)) {
+        return SchemaAge::Unseen;
+    }
     let mut older: bool = false;
     for (relation, columns) in expected {
         let Some(present) = observed.get(*relation) else {

crates/sqlbuild-recall/src/schema/main/version_plan.rs

@@ -3,32 +3,44 @@
 use crate::schema::constants::{RECALL_SCHEMA_VERSION, SECONDS_PER_DAY};
 use crate::schema::models::{SchemaAge, StoreVersion, VersionPlan};
 
-/// Upgrade an older store and record this build's version; never replace a newer store's views or record a lower version than it holds. A version row older than half the retention is appended again with the highest version seen, so retention never removes the last one.
+/// Upgrade an older store and record this build's version; never replace a newer store's views or record a lower version than it holds. A version row older than half the retention is appended again with the highest version seen, so retention never removes the last one. A user who sees no view runs no view statements and, without a version row, records none.
 #[must_use]
 pub fn version_plan(store: StoreVersion, retention_days: u32) -> VersionPlan {
+    let refresh = |version: i64, age_seconds: i64| {
+        (age_seconds.saturating_mul(2) > i64::from(retention_days) * SECONDS_PER_DAY)
+            .then_some(version)
+    };
+    let plan = |replace_views: bool, record: Option<i64>| VersionPlan {
+        replace_views,
+        view_ddl: true,
+        record,
+    };
     match store {
-        StoreVersion::Recorded { version, .. } if version < RECALL_SCHEMA_VERSION => VersionPlan {
-            replace_views: true,
-            record: Some(RECALL_SCHEMA_VERSION),
-        },
         StoreVersion::Recorded {
             version,
             age_seconds,
+            views_visible: false,
         } => VersionPlan {
             replace_views: false,
-            record: (age_seconds.saturating_mul(2) > i64::from(retention_days) * SECONDS_PER_DAY)
-                .then_some(version),
-        },
-        StoreVersion::Unrecorded(SchemaAge::Older) => VersionPlan {
-            replace_views: true,
-            record: Some(RECALL_SCHEMA_VERSION),
-        },
-        StoreVersion::Unrecorded(SchemaAge::Current) => VersionPlan {
-            replace_views: false,
-            record: Some(RECALL_SCHEMA_VERSION),
+            view_ddl: false,
+            record: (version >= RECALL_SCHEMA_VERSION)
+                .then(|| refresh(version, age_seconds))
+                .flatten(),
         },
-        StoreVersion::Unrecorded(SchemaAge::Newer) => VersionPlan {
+        StoreVersion::Recorded { version, .. } if version < RECALL_SCHEMA_VERSION => {
+            plan(true, Some(RECALL_SCHEMA_VERSION))
+        }
+        StoreVersion::Recorded {
+            version,
+            age_seconds,
+            ..
+        } => plan(false, refresh(version, age_seconds)),
+        StoreVersion::Unrecorded(SchemaAge::Older) => plan(true, Some(RECALL_SCHEMA_VERSION)),
+        StoreVersion::Unrecorded(SchemaAge::Current) => plan(false, Some(RECALL_SCHEMA_VERSION)),
+        StoreVersion::Unrecorded(SchemaAge::Newer) => plan(false, None),
+        StoreVersion::Unrecorded(SchemaAge::Unseen) => VersionPlan {
             replace_views: false,
+            view_ddl: false,
             record: None,
         },
     }

crates/sqlbuild-recall/src/schema/models.rs

@@ -128,13 +128,19 @@ pub enum SchemaAge {
     Current,
     /// A relation has a column this build does not know: a newer build created it.
     Newer,
+    /// No view is visible, as to a user granted only the tables, so the store's age is unknown.
+    Unseen,
 }
 
 /// What a writer knows about the store's schema version.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum StoreVersion {
-    /// The highest recorded version, and the age in seconds of the newest version row.
-    Recorded { version: i64, age_seconds: i64 },
+    /// The highest recorded version, the age in seconds of the newest version row, and whether any view is visible to this user.
+    Recorded {
+        version: i64,
+        age_seconds: i64,
+        views_visible: bool,
+    },
     /// No version row is left, as after retention expired it; the columns tell the store's age.
     Unrecorded(SchemaAge),
 }
@@ -144,6 +150,8 @@ pub enum StoreVersion {
 pub struct VersionPlan {
     /// Replace every view with this build's definition.
     pub replace_views: bool,
+    /// Run view statements at all: replacements, or creating missing views. A user who sees no view issues none, since they would fail on every start.
+    pub view_ddl: bool,
     /// The version to append once the views are settled, if any.
     pub record: Option<i64>,
 }

crates/sqlbuild-recall/src/schema/tests/helpers.rs

@@ -119,3 +119,58 @@ pub(crate) fn changed_statements_view(
     statements.retain(|column| !removed.contains(&column.as_str()));
     views
 }
+
+/// Views that pass a table through with `SELECT *`, and that table.
+const PASSTHROUGH_VIEWS: [(&str, Table); 3] = [
+    ("events", Table::Events),
+    ("diagnostics", Table::Diagnostics),
+    ("conflicts", Table::Conflicts),
+];
+
+/// The columns a ClickHouse view body exposes: its passed-through table's, or the aliases of its final top-level `SELECT` list.
+pub(crate) fn clickhouse_view_columns(view: &str, body: &str) -> BTreeSet<String> {
+    PASSTHROUGH_VIEWS
+        .iter()
+        .find(|(name, _)| *name == view)
+        .map(|(_, table)| {
+            table_columns(*table)
+                .into_iter()
+                .map(|column| column.name.to_owned())
+                .collect()
+        })
+        .unwrap_or_else(|| select_list_names(body))
+}
+
+fn select_list_names(body: &str) -> BTreeSet<String> {
+    let start: usize = body.rfind("\nSELECT").map_or(0, |index| index + 1);
+    let list: &str = body[start + "SELECT".len()..]
+        .split("\nFROM ")
+        .next()
+        .unwrap_or_default();
+    top_level_items(list)
+        .iter()
+        .map(|item| item_name(item))
+        .collect()
+}
+
+fn top_level_items(list: &str) -> Vec<String> {
+    let marked: String = list
+        .chars()
+        .scan((0_i32, false), |(depth, quoted), character| {
+            let separator: bool = *depth == 0 && !*quoted && character == ',';
+            *quoted ^= character == '\'';
+            *depth += i32::from(!*quoted && character == '(');
+            *depth -= i32::from(!*quoted && character == ')');
+            Some([character, '\u{0}'][usize::from(separator)])
+        })
+        .collect();
+    marked.split('\u{0}').map(str::to_owned).collect()
+}
+
+fn item_name(item: &str) -> String {
+    let normalized: String = item.split_whitespace().collect::<Vec<&str>>().join(" ");
+    normalized.rsplit_once(" AS ").map_or_else(
+        || normalized.rsplit('.').next().unwrap_or_default().to_owned(),
+        |(_, alias)| alias.to_owned(),
+    )
+}

crates/sqlbuild-recall/src/schema/tests/test_types.rs

@@ -43,3 +43,9 @@ pub(crate) struct VersionPlanTestCase {
     pub(crate) store: crate::schema::models::StoreVersion,
     pub(crate) expected_plan: crate::schema::models::VersionPlan,
 }
+
+pub(crate) struct ViewColumnsTestCase {
+    pub(crate) description: &'static str,
+    pub(crate) view: &'static str,
+    pub(crate) expected_same: bool,
+}