Merge branch 'main' into refactor/planner-native-records

Author: kvlonge

crates/sqlbuild-cli/src/argv/command_table.json

@@ -6853,6 +6853,7 @@
               "json",
               "csv"
              ],
+             "help": "long (default) for reading, json for programs, csv for export; table is deprecated",
              "mutex_group": 0,
              "group": 1
             },
@@ -7017,6 +7018,19 @@
              "help": "with --backend warehouse, describe one relation",
              "group": 1
             },
+            {
+             "kind": "store_true",
+             "options": [
+              "--ddl"
+             ],
+             "dest": "recall_ddl",
+             "nargs": 0,
+             "const": true,
+             "default": false,
+             "help": "print the SQL that creates the Recall store or brings it up to this version",
+             "mutex_group": 0,
+             "group": 1
+            },
             {
              "kind": "store",
              "options": [
@@ -7030,6 +7044,7 @@
               "json",
               "csv"
              ],
+             "help": "long (default) for reading, json for programs, csv for export; table is deprecated",
              "mutex_group": 0,
              "group": 1
             },

crates/sqlbuild-cli/src/argv/constants.rs

@@ -63,7 +63,7 @@ pub(crate) const INT_CONVERTER: &str = "int";
 pub(crate) const PATH_CONVERTER: &str = "Path";
 
 /// Every `CliNamespace` attribute with a default, in declaration order.
-pub(crate) const CLI_NAMESPACE_DEFAULTS: [(&str, NamespaceDefault); 176] = [
+pub(crate) const CLI_NAMESPACE_DEFAULTS: [(&str, NamespaceDefault); 177] = [
     ("command", NamespaceDefault::None),
     ("project_dir", NamespaceDefault::None),
     ("dbt_project_dir", NamespaceDefault::None),
@@ -221,6 +221,7 @@ pub(crate) const CLI_NAMESPACE_DEFAULTS: [(&str, NamespaceDefault); 176] = [
     ("recall_max_cell_bytes", NamespaceDefault::None),
     ("recall_state", NamespaceDefault::Bool(false)),
     ("recall_relation", NamespaceDefault::None),
+    ("recall_ddl", NamespaceDefault::Bool(false)),
     ("recall_invocation", NamespaceDefault::Text("last")),
     ("recall_stream", NamespaceDefault::Text("combined")),
     ("recall_tail", NamespaceDefault::None),

crates/sqlbuild-cli/src/dispatch/constants.rs

@@ -420,7 +420,7 @@ pub(crate) const QUERY_ROUTE: Route = routed(
     },
 );
 
-const RECALL_ARGUMENTS: [Field; 13] = [
+const RECALL_ARGUMENTS: [Field; 14] = [
     PROJECT_DIR,
     SELECTED_TARGET,
     same("recall_command"),
@@ -434,6 +434,7 @@ const RECALL_ARGUMENTS: [Field; 13] = [
     attr("invocation", "recall_invocation"),
     attr("stream", "recall_stream"),
     attr("tail", "recall_tail"),
+    attr("ddl", "recall_ddl"),
 ];
 pub(crate) const RECALL_ROUTE: Route = routed(
     "run_recall",

crates/sqlbuild-cli/src/dispatch/main/invocation_policy.rs

@@ -6,19 +6,21 @@ use crate::dispatch::constants::{
 };
 use crate::dispatch::models::{CommandFacts, InvocationPolicy};
 
-/// Whether the command creates a project, is observed, checks skill freshness and is recorded; only observed commands are recorded, so every recorded invocation has terminal events.
+/// Whether the command creates a project, is observed, checks skill freshness, is recorded and has its streams captured; only observed commands are recorded, so every recorded invocation has terminal events. `sqb recall` only reads history, so it publishes no lifecycle events, runs no sinks and keeps no compute logs.
 #[must_use]
 pub fn invocation_policy(facts: &CommandFacts) -> InvocationPolicy {
     let command: &str = facts.command.as_deref().unwrap_or_default();
     let creates_project: bool = PROJECT_CREATING_COMMANDS.contains(&command)
         || (command == DBT_COMMAND_NAME
             && facts.subcommand.as_deref() == Some(DBT_INIT_SUBCOMMAND));
     let raw_query_diff: bool = command == DIFF_COMMAND_NAME && facts.raw_query_diff;
-    let observed: bool = !creates_project && !raw_query_diff;
+    let reads_history: bool = command == RECALL_COMMAND_NAME;
+    let observed: bool = !creates_project && !raw_query_diff && !reads_history;
     InvocationPolicy {
         creates_project,
         observed,
         maintains_skills: !SKILL_EXEMPT_COMMANDS.contains(&command) && !raw_query_diff,
-        recalled: observed && !command.is_empty() && command != RECALL_COMMAND_NAME,
+        recalled: observed && !command.is_empty(),
+        captured: !creates_project && !reads_history,
     }
 }

crates/sqlbuild-cli/src/dispatch/models.rs

@@ -121,6 +121,8 @@ pub struct InvocationPolicy {
     pub maintains_skills: bool,
     /// Whether Recall records the invocation; `sqb recall` never records itself.
     pub recalled: bool,
+    /// Whether compute logs capture the process streams; read-only `sqb recall` skips them.
+    pub captured: bool,
 }
 
 /// What a failed command reports: its fallback error code and the debug log message.

crates/sqlbuild-cli/src/dispatch/tests/test_invocation_policy.rs

@@ -14,6 +14,7 @@ fn given_parsed_command_when_choosing_services_then_matches_python_entry_rules()
                 observed: true,
                 maintains_skills: true,
                 recalled: true,
+                captured: true,
             },
         },
         InvocationPolicyTestCase {
@@ -24,6 +25,7 @@ fn given_parsed_command_when_choosing_services_then_matches_python_entry_rules()
                 observed: false,
                 maintains_skills: true,
                 recalled: false,
+                captured: false,
             },
         },
         InvocationPolicyTestCase {
@@ -34,6 +36,7 @@ fn given_parsed_command_when_choosing_services_then_matches_python_entry_rules()
                 observed: false,
                 maintains_skills: false,
                 recalled: false,
+                captured: false,
             },
         },
         InvocationPolicyTestCase {
@@ -47,16 +50,18 @@ fn given_parsed_command_when_choosing_services_then_matches_python_entry_rules()
                 observed: false,
                 maintains_skills: false,
                 recalled: false,
+                captured: true,
             },
         },
         InvocationPolicyTestCase {
-            description: "recall reads history and is never recorded itself",
+            description: "recall only reads history: no events, sinks, recording or compute logs",
             facts: facts("recall", Some("query")),
             expected_policy: InvocationPolicy {
                 creates_project: false,
-                observed: true,
+                observed: false,
                 maintains_skills: true,
                 recalled: false,
+                captured: false,
             },
         },
     ];

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

@@ -2,14 +2,17 @@
 
 pub const RECALL_SECTION: &str = "recall";
 /// Keys `[recall]` and `targets.<name>.recall` accept, sorted.
-pub const RECALL_KEYS: [&str; 6] = [
-    "connection",
+pub const RECALL_KEYS: [&str; 7] = [
     "diagnostics_min_severity",
     "enabled",
     "max_output_bytes",
+    "read_connection",
     "retention",
     "shutdown_timeout",
+    "write_connection",
 ];
+/// The single store key `write_connection` and `read_connection` replaced.
+pub const REMOVED_CONNECTION_KEY: &str = "connection";
 /// The diagnostic severities a Recall store can keep from, least severe first.
 pub const RECALL_SEVERITIES: [&str; 6] = ["trace", "debug", "info", "warning", "error", "critical"];
 /// The longest shutdown flush budget, in milliseconds.

crates/sqlbuild-config/src/recall/main/recall_section.rs

@@ -2,7 +2,9 @@
 
 use crate::models::ConfigValue;
 use crate::recall::_helpers::values::{non_empty, output_bytes, retention_days, timeout_ms};
-use crate::recall::constants::{MAX_RETENTION_DAYS, RECALL_KEYS, RECALL_SEVERITIES};
+use crate::recall::constants::{
+    MAX_RETENTION_DAYS, RECALL_KEYS, RECALL_SEVERITIES, REMOVED_CONNECTION_KEY,
+};
 use crate::recall::models::RecallSection;
 
 /// The settings of `value`, or the message naming its first problem at `label`.
@@ -20,7 +22,13 @@ pub fn recall_section(value: &ConfigValue, label: &str) -> Result<RecallSection,
                 ConfigValue::Bool(flag) => section.enabled = Some(*flag),
                 _ => return Err(format!("{label}.enabled must be true or false")),
             },
-            "connection" => section.connection = Some(non_empty(value, label, key)?.to_owned()),
+            "write_connection" => {
+                section.write_connection = Some(non_empty(value, label, key)?.to_owned());
+            }
+            "read_connection" => {
+                section.read_connection = Some(non_empty(value, label, key)?.to_owned());
+            }
+            REMOVED_CONNECTION_KEY => return Err(removed_connection(value, label)),
             "retention" => {
                 let text = non_empty(value, label, key)?;
                 section.retention_days = Some(retention_days(text).ok_or_else(|| {
@@ -67,3 +75,17 @@ pub fn recall_section(value: &ConfigValue, label: &str) -> Result<RecallSection,
     }
     Ok(section)
 }
+
+/// The message for the removed `connection` key, naming its replacements.
+fn removed_connection(value: &ConfigValue, label: &str) -> String {
+    let name: String = match value {
+        ConfigValue::String(name) if !name.trim().is_empty() => name.trim().to_owned(),
+        _ => "<name>".to_owned(),
+    };
+    format!(
+        "{label}.connection was replaced by write_connection and read_connection: set \
+         write_connection = \"{name}\" and read_connection = \"{name}\" (or a separate \
+         read-only connection), or only read_connection to read shared history while this \
+         machine records to the local store"
+    )
+}

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

@@ -4,7 +4,10 @@
 #[derive(Clone, Debug, Default, PartialEq, Eq)]
 pub struct RecallSection {
     pub enabled: Option<bool>,
-    pub connection: Option<String>,
+    /// The connection every command writes its history to; the merged settings must also set `read_connection`.
+    pub write_connection: Option<String>,
+    /// The connection `sqb recall query`, `schema` and `output` read.
+    pub read_connection: Option<String>,
     pub retention_days: Option<u32>,
     pub shutdown_timeout_ms: Option<u64>,
     pub diagnostics_min_severity: Option<String>,

crates/sqlbuild-config/src/recall/tests/test_recall_section.rs

@@ -11,21 +11,31 @@ fn given_valid_sections_when_validated_then_settings_are_typed() {
             description: "every key is typed and trimmed",
             value: table(&[
                 ("enabled", ConfigValue::Bool(false)),
-                ("connection", text(" history ")),
+                ("write_connection", text(" history_writer ")),
+                ("read_connection", text("history_reader")),
                 ("retention", text("720h")),
                 ("shutdown_timeout", text("1500ms")),
                 ("diagnostics_min_severity", text("debug")),
                 ("max_output_bytes", ConfigValue::Integer(4_194_304)),
             ]),
             expected_section: RecallSection {
                 enabled: Some(false),
-                connection: Some("history".to_owned()),
+                write_connection: Some("history_writer".to_owned()),
+                read_connection: Some("history_reader".to_owned()),
                 retention_days: Some(30),
                 shutdown_timeout_ms: Some(1500),
                 diagnostics_min_severity: Some("debug".to_owned()),
                 max_output_bytes: Some(4_194_304),
             },
         },
+        ValidSectionTestCase {
+            description: "a read connection alone reads shared history",
+            value: table(&[("read_connection", text("history_reader"))]),
+            expected_section: RecallSection {
+                read_connection: Some("history_reader".to_owned()),
+                ..RecallSection::default()
+            },
+        },
         ValidSectionTestCase {
             description: "zero output bytes means record everything",
             value: table(&[("max_output_bytes", ConfigValue::Integer(0))]),
@@ -66,6 +76,14 @@ fn given_invalid_sections_when_validated_then_the_message_names_the_key() {
             label: "recall",
             expected_message_prefix: "recall.max_output_bytes must be",
         },
+        InvalidSectionTestCase {
+            description: "the removed connection key names its replacements",
+            value: table(&[("connection", text("history"))]),
+            label: "targets.prod.recall",
+            expected_message_prefix: "targets.prod.recall.connection was replaced by write_connection \
+                                      and read_connection: set write_connection = \"history\" and \
+                                      read_connection = \"history\"",
+        },
         InvalidSectionTestCase {
             description: "unknown keys are listed",
             value: table(&[("mode", text("x"))]),