From 979f6825d827e830fd17f49e72cdd152e423e0d2 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:35:31 +0000 Subject: [PATCH 1/8] refactor: keep SQL-test cache and result processing native Preserve existing cache snapshots and Python APIs while moving cache persistence and completed-test result consumers to native ownership. Amp-Thread-ID: https://ampcode.com/threads/T-01a18715-4e00-708e-a87d-6bd109748d66 --- .../sqlbuild-cache/src/artifacts/constants.rs | 5 + .../sqlbuild-cache/src/artifacts/main/mod.rs | 3 +- .../src/artifacts/main/read_sql_test_cache.rs | 66 ++++++ .../artifacts/main/stored_sql_test_record.rs | 24 -- .../artifacts/main/write_sql_test_cache.rs | 64 ++++++ crates/sqlbuild-cache/src/artifacts/models.rs | 11 - .../artifacts/tests/test_reusable_record.rs | 129 ++++++++++- .../src/artifacts/tests/test_types.rs | 14 ++ .../_helpers/build_progress/result_text.rs | 31 ++- .../_helpers/compile_command/target_writer.rs | 6 +- .../compile_command/test_artifact_cache.rs | 210 ++++-------------- .../_helpers/compile_outputs/json_report.rs | 16 -- .../compile_outputs/runtime_target.rs | 18 +- .../_helpers/result_output/documents.rs | 12 +- .../_helpers/sql_test_command/test_command.rs | 36 ++- fensu.toml | 5 +- .../cli/commands/main/test_compile.py | 34 ++- 17 files changed, 415 insertions(+), 269 deletions(-) create mode 100644 crates/sqlbuild-cache/src/artifacts/main/read_sql_test_cache.rs delete mode 100644 crates/sqlbuild-cache/src/artifacts/main/stored_sql_test_record.rs create mode 100644 crates/sqlbuild-cache/src/artifacts/main/write_sql_test_cache.rs diff --git a/crates/sqlbuild-cache/src/artifacts/constants.rs b/crates/sqlbuild-cache/src/artifacts/constants.rs index f55814e80a..93d990ca58 100644 --- a/crates/sqlbuild-cache/src/artifacts/constants.rs +++ b/crates/sqlbuild-cache/src/artifacts/constants.rs @@ -1,5 +1,10 @@ /// The mode new artifacts are created with before the process umask, as Python's `os.open`. pub const NEW_FILE_MODE: u32 = 0o666; +/// The existing SQL-test cache snapshot format, shared by native readers and writers. +pub(crate) const SQL_TEST_CACHE_VERSION: u64 = 5; +pub(crate) const SQL_TEST_CACHE_FILE: &str = "sql-test-artifacts.json"; +pub(crate) const MAX_SQL_TEST_CACHE_BYTES: u64 = 10_000_000; +pub(crate) const MAX_SQL_TEST_CACHE_RECORDS: usize = 100_000; /// The extension every compiled SQL test artifact carries. pub(crate) const SQL_EXTENSION: &str = "sql"; #[cfg(unix)] diff --git a/crates/sqlbuild-cache/src/artifacts/main/mod.rs b/crates/sqlbuild-cache/src/artifacts/main/mod.rs index 1f3a4d7571..d197b433a3 100644 --- a/crates/sqlbuild-cache/src/artifacts/main/mod.rs +++ b/crates/sqlbuild-cache/src/artifacts/main/mod.rs @@ -1,8 +1,9 @@ pub mod new_sql_test_record; pub mod publish_staged; +pub mod read_sql_test_cache; pub mod remove_stale; pub mod remove_stale_unless_unchanged; pub mod reusable_sql_test_record; pub mod staged_files; -pub mod stored_sql_test_record; pub mod write_artifacts; +pub mod write_sql_test_cache; diff --git a/crates/sqlbuild-cache/src/artifacts/main/read_sql_test_cache.rs b/crates/sqlbuild-cache/src/artifacts/main/read_sql_test_cache.rs new file mode 100644 index 0000000000..61c2d6046c --- /dev/null +++ b/crates/sqlbuild-cache/src/artifacts/main/read_sql_test_cache.rs @@ -0,0 +1,66 @@ +//! Read the persisted SQL-test artifact records without an interpreter. + +use std::collections::HashMap; +use std::path::Path; + +use serde_json::Value; + +use crate::artifacts::_helpers::sql_test_records::{ + is_safe_relative_sql_path, normalized_relative_path, +}; +use crate::artifacts::constants::{ + MAX_SQL_TEST_CACHE_BYTES, MAX_SQL_TEST_CACHE_RECORDS, SQL_TEST_CACHE_FILE, + SQL_TEST_CACHE_VERSION, +}; +use crate::artifacts::models::SqlTestArtifactRecord; + +/// Valid records in the existing snapshot; an unreadable or malformed cache is a miss. +#[must_use] +pub fn read_sql_test_cache(directory: &Path) -> HashMap { + let path = directory.join(SQL_TEST_CACHE_FILE); + let Ok(metadata) = std::fs::metadata(&path) else { + return HashMap::new(); + }; + if !metadata.is_file() || metadata.len() > MAX_SQL_TEST_CACHE_BYTES { + return HashMap::new(); + } + let Ok(bytes) = std::fs::read(&path) else { + return HashMap::new(); + }; + let Ok(payload) = serde_json::from_slice::(&bytes) else { + return HashMap::new(); + }; + if payload.get("version").and_then(Value::as_f64) != Some(SQL_TEST_CACHE_VERSION as f64) { + return HashMap::new(); + } + let Some(rows) = payload.get("records").and_then(Value::as_object) else { + return HashMap::new(); + }; + if rows.len() > MAX_SQL_TEST_CACHE_RECORDS { + return HashMap::new(); + } + let mut records: HashMap = HashMap::new(); + for (key, value) in rows { + if let Some(record) = record(value) { + records.insert(key.clone(), record); + } + } + records +} + +fn record(value: &Value) -> Option { + let relative_path = normalized_relative_path(Path::new(value.get("relative_path")?.as_str()?)); + if !is_safe_relative_sql_path(&relative_path) { + return None; + } + let Ok(mtime_ns) = i64::try_from(value.get("mtime_ns")?.as_u64()?) else { + return None; + }; + Some(SqlTestArtifactRecord { + identity: value.get("identity")?.as_str()?.to_owned(), + relative_path, + size: value.get("size")?.as_u64()?, + mtime_ns, + content_sha256: value.get("content_sha256")?.as_str()?.to_owned(), + }) +} diff --git a/crates/sqlbuild-cache/src/artifacts/main/stored_sql_test_record.rs b/crates/sqlbuild-cache/src/artifacts/main/stored_sql_test_record.rs deleted file mode 100644 index 896d5a812d..0000000000 --- a/crates/sqlbuild-cache/src/artifacts/main/stored_sql_test_record.rs +++ /dev/null @@ -1,24 +0,0 @@ -//! A SQL test artifact record read back from the cache file. - -use std::path::{Path, PathBuf}; - -use crate::artifacts::_helpers::sql_test_records::{ - is_safe_relative_sql_path, normalized_relative_path, -}; -use crate::artifacts::models::{SqlTestArtifactRecord, StoredSqlTestRecord}; - -/// The stored record, or `None` when its path could leave the tests directory. -#[must_use] -pub fn stored_sql_test_record(stored: StoredSqlTestRecord) -> Option { - let relative_path: PathBuf = normalized_relative_path(Path::new(&stored.relative_path)); - if !is_safe_relative_sql_path(&relative_path) { - return None; - } - Some(SqlTestArtifactRecord { - identity: stored.identity, - relative_path, - size: stored.size, - mtime_ns: stored.mtime_ns, - content_sha256: stored.content_sha256, - }) -} diff --git a/crates/sqlbuild-cache/src/artifacts/main/write_sql_test_cache.rs b/crates/sqlbuild-cache/src/artifacts/main/write_sql_test_cache.rs new file mode 100644 index 0000000000..8dec31b77b --- /dev/null +++ b/crates/sqlbuild-cache/src/artifacts/main/write_sql_test_cache.rs @@ -0,0 +1,64 @@ +//! Publish the existing SQL-test cache snapshot directly from native records. + +use std::collections::BTreeMap; +use std::path::Path; + +use sqlbuild_core::json::main::dumps::dumps; +use sqlbuild_core::json::models::{JsonDialect, JsonValue, StdlibJsonOptions}; + +use crate::artifacts::constants::{SQL_TEST_CACHE_FILE, SQL_TEST_CACHE_VERSION}; +use crate::artifacts::models::SqlTestArtifactRecord; + +/// Atomically replace the snapshot with Python-compatible JSON; write failures are cache misses. +pub fn write_sql_test_cache(directory: &Path, records: &[(String, SqlTestArtifactRecord)]) { + let rows: BTreeMap = records + .iter() + .map(|(key, record)| { + let fields = JsonValue::Object(vec![ + ( + "identity".to_owned(), + JsonValue::String(record.identity.clone()), + ), + ( + "relative_path".to_owned(), + JsonValue::String(record.relative_posix()), + ), + ("size".to_owned(), JsonValue::Integer(record.size.into())), + ( + "mtime_ns".to_owned(), + JsonValue::Integer(record.mtime_ns.into()), + ), + ( + "content_sha256".to_owned(), + JsonValue::String(record.content_sha256.clone()), + ), + ]); + (key.clone(), fields) + }) + .collect(); + let payload = JsonValue::Object(vec![ + ( + "version".to_owned(), + JsonValue::Integer(SQL_TEST_CACHE_VERSION.into()), + ), + ( + "records".to_owned(), + JsonValue::Object(rows.into_iter().collect()), + ), + ]); + let Ok(encoded) = dumps( + &payload, + &JsonDialect::Stdlib(StdlibJsonOptions { + sort_keys: true, + ..StdlibJsonOptions::new(None).with_separators(",", ":") + }), + ) else { + return; + }; + let temporary = directory.join(format!(".{SQL_TEST_CACHE_FILE}.{}.tmp", std::process::id())); + let _ = (|| -> std::io::Result<()> { + std::fs::create_dir_all(directory)?; + std::fs::write(&temporary, encoded.as_bytes())?; + std::fs::rename(&temporary, directory.join(SQL_TEST_CACHE_FILE)) + })(); +} diff --git a/crates/sqlbuild-cache/src/artifacts/models.rs b/crates/sqlbuild-cache/src/artifacts/models.rs index fb8d1c2a05..ef2d8bd714 100644 --- a/crates/sqlbuild-cache/src/artifacts/models.rs +++ b/crates/sqlbuild-cache/src/artifacts/models.rs @@ -59,17 +59,6 @@ impl SqlTestRecordMiss { } } -/// The fields of one cache file record, already checked to be strings and non-negative integers. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct StoredSqlTestRecord { - pub identity: String, - /// The `relative_path` text as stored. - pub relative_path: String, - pub size: u64, - pub mtime_ns: i64, - pub content_sha256: String, -} - impl SqlTestArtifactRecord { /// The relative path's `as_posix()` text, as the cache file stores it. #[must_use] diff --git a/crates/sqlbuild-cache/src/artifacts/tests/test_reusable_record.rs b/crates/sqlbuild-cache/src/artifacts/tests/test_reusable_record.rs index e7037a63f9..b36bdda965 100644 --- a/crates/sqlbuild-cache/src/artifacts/tests/test_reusable_record.rs +++ b/crates/sqlbuild-cache/src/artifacts/tests/test_reusable_record.rs @@ -1,8 +1,12 @@ use crate::artifacts::main::new_sql_test_record::new_sql_test_record; +use crate::artifacts::main::read_sql_test_cache::read_sql_test_cache; use crate::artifacts::main::reusable_sql_test_record::reusable_sql_test_record; -use crate::artifacts::models::SqlTestRecordMiss; +use crate::artifacts::main::write_sql_test_cache::write_sql_test_cache; +use crate::artifacts::models::{SqlTestArtifactRecord, SqlTestRecordMiss}; use crate::artifacts::tests::helpers::{bump_mtime, place}; -use crate::artifacts::tests::test_types::ReusableRecordTestCase; +use crate::artifacts::tests::test_types::{ + ReusableRecordTestCase, SqlTestSnapshotReadTestCase, SqlTestSnapshotWriteTestCase, +}; const IDENTITY: &str = "identity-1"; @@ -66,3 +70,124 @@ fn given_recorded_artifact_when_checking_reuse_then_reports_why_it_missed() { ); } } + +#[test] +fn given_native_records_when_persisting_then_preserves_python_snapshot_bytes_and_reuse() { + let test_cases = [SqlTestSnapshotWriteTestCase { + description: "sorted Unicode records, exact large integers and last duplicate wins", + expected_text: concat!( + r#"{"records":{"a":{"content_sha256":"abc","identity":"caf\u00e9 \ud83d\ude00","mtime_ns":9223372036854775807,"relative_path":"nested/orders.sql","size":18446744073709551615},"#, + r#""z":{"content_sha256":"abc","identity":"caf\u00e9 \ud83d\ude00","mtime_ns":9223372036854775807,"relative_path":"nested/orders.sql","size":7}},"version":5}"#, + ), + expected_record_count: 2, + }]; + for test_case in test_cases { + let root = tempfile::tempdir().expect("cache folder"); + let record = SqlTestArtifactRecord { + identity: "café 😀".to_owned(), + relative_path: "nested/orders.sql".into(), + size: u64::MAX, + mtime_ns: i64::MAX, + content_sha256: "abc".to_owned(), + }; + let records = vec![ + ("z".to_owned(), record.clone()), + ("a".to_owned(), record.clone()), + ( + "z".to_owned(), + SqlTestArtifactRecord { + size: 7, + ..record.clone() + }, + ), + ]; + + write_sql_test_cache(root.path(), &records); + + let path = root.path().join("sql-test-artifacts.json"); + let text = std::fs::read_to_string(&path).expect("written snapshot"); + assert_eq!(text, test_case.expected_text, "{}", test_case.description); + let loaded = read_sql_test_cache(root.path()); + assert_eq!( + loaded.len(), + test_case.expected_record_count, + "{}", + test_case.description + ); + assert_eq!(loaded["a"], record, "{}", test_case.description); + assert_eq!(loaded["z"].size, 7, "{}", test_case.description); + + std::fs::remove_file(&path).expect("remove snapshot"); + std::fs::create_dir(&path).expect("blocked destination"); + write_sql_test_cache(root.path(), &records); + assert!(path.is_dir(), "{}", test_case.description); + } +} + +#[test] +fn given_mixed_or_broken_snapshots_when_reading_then_keeps_only_safe_well_typed_records() { + let test_cases = [ + SqlTestSnapshotReadTestCase { + description: "empty snapshot", + contents: "", + expected_records: &[], + }, + SqlTestSnapshotReadTestCase { + description: "truncated JSON", + contents: "{", + expected_records: &[], + }, + SqlTestSnapshotReadTestCase { + description: "non-object root", + contents: "[]", + expected_records: &[], + }, + SqlTestSnapshotReadTestCase { + description: "wrong version", + contents: r#"{"version":6,"records":{}}"#, + expected_records: &[], + }, + SqlTestSnapshotReadTestCase { + description: "non-object records", + contents: r#"{"version":5,"records":[]}"#, + expected_records: &[], + }, + SqlTestSnapshotReadTestCase { + description: "valid record survives bad types, ranges and paths; numeric version is compatible", + contents: r#"{"version":5.0,"records":{ + "valid":{"identity":"orders","relative_path":"nested/orders.sql","size":11,"mtime_ns":123,"content_sha256":"abc"}, + "bool":{"identity":"orders","relative_path":"orders.sql","size":true,"mtime_ns":123,"content_sha256":"abc"}, + "float":{"identity":"orders","relative_path":"orders.sql","size":11.0,"mtime_ns":123,"content_sha256":"abc"}, + "negative":{"identity":"orders","relative_path":"orders.sql","size":-1,"mtime_ns":123,"content_sha256":"abc"}, + "overflow":{"identity":"orders","relative_path":"orders.sql","size":11,"mtime_ns":9223372036854775808,"content_sha256":"abc"}, + "type":{"identity":17,"relative_path":"orders.sql","size":11,"mtime_ns":123,"content_sha256":"abc"}, + "escape":{"identity":"orders","relative_path":"../outside.sql","size":11,"mtime_ns":123,"content_sha256":"abc"}, + "extension":{"identity":"orders","relative_path":"orders.txt","size":11,"mtime_ns":123,"content_sha256":"abc"} + }}"#, + expected_records: &[("orders", 11)], + }, + ]; + for test_case in test_cases { + let root = tempfile::tempdir().expect("cache folder"); + assert!( + read_sql_test_cache(root.path()).is_empty(), + "{}", + test_case.description + ); + std::fs::write( + root.path().join("sql-test-artifacts.json"), + test_case.contents, + ) + .expect("snapshot"); + let loaded = read_sql_test_cache(root.path()); + let actual: Vec<(&str, u64)> = loaded + .values() + .map(|record| (record.identity.as_str(), record.size)) + .collect(); + assert_eq!( + actual, test_case.expected_records, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-cache/src/artifacts/tests/test_types.rs b/crates/sqlbuild-cache/src/artifacts/tests/test_types.rs index 12583ab0a3..9a5f782e61 100644 --- a/crates/sqlbuild-cache/src/artifacts/tests/test_types.rs +++ b/crates/sqlbuild-cache/src/artifacts/tests/test_types.rs @@ -57,3 +57,17 @@ pub(crate) struct ReusableRecordTestCase { pub(crate) identity: &'static str, pub(crate) expected_outcome: Result<(), crate::artifacts::models::SqlTestRecordMiss>, } + +/// A cache snapshot's externally visible encoding and recovered records. +pub(crate) struct SqlTestSnapshotWriteTestCase { + pub(crate) description: &'static str, + pub(crate) expected_text: &'static str, + pub(crate) expected_record_count: usize, +} + +/// A disposable on-disk snapshot, including absent and malformed files. +pub(crate) struct SqlTestSnapshotReadTestCase { + pub(crate) description: &'static str, + pub(crate) contents: &'static str, + pub(crate) expected_records: &'static [(&'static str, u64)], +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/build_progress/result_text.rs b/crates/sqlbuild-python/src/bindings/_helpers/build_progress/result_text.rs index 699a622437..f3d9ab4dfc 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/build_progress/result_text.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/build_progress/result_text.rs @@ -279,24 +279,31 @@ fn test_step_text(step: &StepResult) -> TestStepText { fn test_result_text(test: &SqlTestExecutionResult>) -> PyResult { Ok(TestResultText { - label: test_label_values( - &test.test_name, - test.source_path - .as_ref() - .map(|path| { - path.to_string_lossy() - .replace(std::path::MAIN_SEPARATOR, "/") - }) - .as_deref(), - &test.parameter_schema, - &test.parameter_values, - )?, + label: test_result_label(test)?, outcome: test.outcome.value().to_owned(), steps: test.step_results.iter().map(test_step_text).collect(), error: error(&test.error_code, &test.error_message, &test.error_help), }) } +/// The shared progress/footer/command label of an already-converted executor test result. +pub(crate) fn test_result_label( + test: &SqlTestExecutionResult>, +) -> PyResult { + test_label_values( + &test.test_name, + test.source_path + .as_ref() + .map(|path| { + path.to_string_lossy() + .replace(std::path::MAIN_SEPARATOR, "/") + }) + .as_deref(), + &test.parameter_schema, + &test.parameter_values, + ) +} + pub(crate) fn python_node_text(node: &Bound<'_, PyAny>) -> PyResult { Ok(PythonNodeText { name: text(node, "node_name")?, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs index 6045989c26..7432a8e859 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs @@ -433,7 +433,7 @@ fn planned_artifacts<'py>( /// Each test's cache slot and artifact identity, hashed together without holding the interpreter. fn test_identities<'py>( tests: &[Bound<'py, PyAny>], - context: &IdentityContext<'py>, + context: &IdentityContext, chains: &Bound<'py, PyAny>, ) -> PyResult> { let Some(first) = tests.first() else { @@ -482,10 +482,10 @@ fn plan_tests<'py>( let cache_dir: Bound<'py, PyAny> = native_field(project, "compile_cache_dir")?; let cached: HashMap = read_cache(&cache_dir)?; let tests: Bound<'py, PyAny> = native_field(project, "sql_tests")?; - let caching: Option<(IdentityContext<'py>, Bound<'py, PyAny>)> = if cache_dir.is_none() { + let caching: Option<(IdentityContext, Bound<'py, PyAny>)> = if cache_dir.is_none() { None } else { - let context: IdentityContext<'py> = identity_context(project, adapter)?; + let context: IdentityContext = identity_context(project, adapter)?; count_stage(py, SQL_TEST_MODEL_CHAINS, 1)?; let chains: Bound<'py, PyAny> = sql_test_model_chain_names_by_key(project, &tests)?.into_any(); diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs index b815114b3a..77e3419d92 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs @@ -5,31 +5,28 @@ use std::path::PathBuf; use pyo3::IntoPyObjectExt; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python}; -use pyo3::types::{PyBool, PyDict, PyDictMethods, PyInt, PyList, PyListMethods, PyString, PyTuple}; +use pyo3::types::{PyDict, PyDictMethods, PyList, PyListMethods, PyTuple}; use rayon::prelude::{IntoParallelRefIterator, ParallelIterator}; -use sqlbuild_cache::artifacts::main::stored_sql_test_record::stored_sql_test_record; -use sqlbuild_cache::artifacts::models::{SqlTestArtifactRecord, StoredSqlTestRecord}; +use sqlbuild_cache::artifacts::main::read_sql_test_cache::read_sql_test_cache; +use sqlbuild_cache::artifacts::main::write_sql_test_cache::write_sql_test_cache; +use sqlbuild_cache::artifacts::models::SqlTestArtifactRecord; use sqlbuild_cache::digest::main::bytes_digest::bytes_digest; use sqlbuild_cache::digest::main::hex_digest::hex_digest; use crate::bindings::_helpers::boundary::python_calls::imported; -use crate::bindings::_helpers::compile_outputs::json_report::{compact_sorted_json, orjson_sorted}; +use crate::bindings::_helpers::compile_outputs::json_report::orjson_sorted; use crate::bindings::_helpers::records::construct::record_class; use crate::bindings::_helpers::records::ids::RecordId; -const CACHE_VERSION: i64 = 5; const ALGORITHM_FINGERPRINT: &str = "sql-test-artifact-v5-orjson-blake3-identity"; /// Items a loop holding the interpreter handles before letting other threads run. const INTERPRETER_YIELD_INTERVAL: usize = 64; -const CACHE_FILE_NAME: &str = "sql-test-artifacts.json"; -const MAX_CACHE_BYTES: u64 = 10_000_000; -const MAX_CACHE_RECORDS: usize = 100_000; const METADATA_MODULE: &str = "importlib.metadata"; /// The digest shared by every test artifact identity, and each model's own identity. -pub(crate) struct IdentityContext<'py> { +pub(crate) struct IdentityContext { common_identity: String, - model_identities: Bound<'py, PyDict>, + model_identities: HashMap, } /// `orjson.dumps(payload, option=OPT_SORT_KEYS)`, the text an identity digest is taken of. @@ -65,13 +62,10 @@ fn dict<'py>( Ok(built) } -/// `sorted(items)`, as Python orders them. -fn sorted<'py>(items: Bound<'py, PyList>) -> PyResult> { - items - .py() - .import("builtins")? - .getattr("sorted")? - .call1((items,)) +/// String pairs ordered lexicographically, as Python orders these cache inputs. +fn sorted_pairs(py: Python<'_>, mut items: Vec<(String, String)>) -> PyResult> { + items.sort(); + Ok(PyList::new(py, items)?.into_any()) } fn package_version<'py>(py: Python<'py>, package: &str) -> PyResult> { @@ -89,12 +83,15 @@ fn package_version<'py>(py: Python<'py>, package: &str) -> PyResult(deps: &Bound<'py, PyAny>) -> PyResult> { let py: Python<'py> = deps.py(); - let pairs: Bound<'py, PyList> = PyList::empty(py); + let mut pairs: Vec<(String, String)> = Vec::new(); for dep in deps.try_iter()? { let dep: Bound<'py, PyAny> = dep?; - pairs.append((dep.getattr("resource_type")?.str()?, dep.getattr("name")?))?; + pairs.push(( + dep.getattr("resource_type")?.str()?.extract()?, + dep.getattr("name")?.extract()?, + )); } - sorted(pairs) + sorted_pairs(py, pairs) } fn model_identity_payload(model: &Bound<'_, PyAny>) -> PyResult { @@ -125,16 +122,19 @@ fn model_identity_payload(model: &Bound<'_, PyAny>) -> PyResult { pub(crate) fn identity_context<'py>( project: &Bound<'py, PyAny>, adapter: &Bound<'py, PyAny>, -) -> PyResult> { +) -> PyResult { let py: Python<'py> = project.py(); let adapter_type: Bound<'py, PyAny> = adapter.get_type().into_any(); - let functions: Bound<'py, PyList> = PyList::empty(py); + let mut functions: Vec<(String, String)> = Vec::new(); for function in project.getattr("functions")?.try_iter()? { let function: Bound<'py, PyAny> = function?; - functions.append(( - function.getattr("name")?, - function.getattr("destination")?.getattr("qualified_name")?, - ))?; + functions.push(( + function.getattr("name")?.extract()?, + function + .getattr("destination")? + .getattr("qualified_name")? + .extract()?, + )); } let version_tuple: Bound<'py, PyAny> = py .import("platform")? @@ -187,24 +187,21 @@ pub(crate) fn identity_context<'py>( "effective_target_schema", project.getattr("effective_target_schema")?, ), - ("function_locations", sorted(functions)?), + ("function_locations", sorted_pairs(py, functions)?), ], )?; - let mut names: Vec> = Vec::new(); + let mut names: Vec = Vec::new(); let mut payloads: Vec = Vec::new(); for (index, model) in project.getattr("models")?.try_iter()?.enumerate() { let model: Bound<'py, PyAny> = model?; - names.push(model.getattr("name")?); + names.push(model.getattr("name")?.extract()?); payloads.push(model_identity_payload(&model)?); yield_interpreter(py, index); } payloads.push(payload_text(common.as_any())?); let mut digests: Vec = payload_digests(py, &payloads); let common_identity: String = digests.pop().unwrap_or_default(); - let model_identities: Bound<'py, PyDict> = PyDict::new(py); - for (name, digest) in names.into_iter().zip(digests) { - model_identities.set_item(name, digest)?; - } + let model_identities: HashMap = names.into_iter().zip(digests).collect(); Ok(IdentityContext { common_identity, model_identities, @@ -225,13 +222,14 @@ fn cte_payload<'py>(ctes: &Bound<'py, PyAny>) -> PyResult> { /// `sorted(mapping.items())`. fn sorted_items<'py>(mapping: &Bound<'py, PyAny>) -> PyResult> { let py: Python<'py> = mapping.py(); - sorted(PyList::new( + sorted_pairs( py, mapping .call_method0("items")? .try_iter()? + .map(|item| item?.extract::<(String, String)>()) .collect::>>()?, - )?) + ) } fn test_payload<'py>(test: &Bound<'py, PyAny>) -> PyResult> { @@ -321,13 +319,13 @@ fn test_payload<'py>(test: &Bound<'py, PyAny>) -> PyResult> { pub(crate) fn artifact_identity_payload( test: &Bound<'_, PyAny>, model_chain_names: &Bound<'_, PyAny>, - context: &IdentityContext<'_>, + context: &IdentityContext, ) -> PyResult { let py: Python<'_> = test.py(); let chain: Bound<'_, PyList> = PyList::empty(py); for name in model_chain_names.try_iter()? { - let name: Bound<'_, PyAny> = name?; - let identity: Option> = context.model_identities.get_item(&name)?; + let name: String = name?.extract()?; + let identity: Option<&String> = context.model_identities.get(&name); chain.append((name, identity))?; } payload_text( @@ -373,58 +371,6 @@ pub(crate) fn artifact_record_key_payload(test: &Bound<'_, PyAny>) -> PyResult, key: &str) -> PyResult> { - match fields.get_item(key)? { - Some(item) if item.is_instance_of::() => Ok(Some(item.extract()?)), - _ => Ok(None), - } -} - -/// A non-negative integer field of a stored record, or `None` when absent, negative or not an `int`. -fn stored_integer(fields: &Bound<'_, PyDict>, key: &str) -> PyResult> { - let Some(item) = fields.get_item(key)? else { - return Ok(None); - }; - if !item.is_instance_of::() || item.is_instance_of::() { - return Ok(None); - } - let Ok(number) = item.extract::() else { - return Ok(None); - }; - Ok(Some(number)) -} - -/// One stored record, or `None` when any field has the wrong type or an unsafe path. -fn stored_record(value: &Bound<'_, PyAny>) -> PyResult> { - let Ok(fields) = value.downcast::() else { - return Ok(None); - }; - let (Some(identity), Some(relative_path), Some(content_sha256)) = ( - stored_text(fields, "identity")?, - stored_text(fields, "relative_path")?, - stored_text(fields, "content_sha256")?, - ) else { - return Ok(None); - }; - let (Some(size), Some(mtime_ns)) = ( - stored_integer(fields, "size")?, - stored_integer(fields, "mtime_ns")?, - ) else { - return Ok(None); - }; - let Ok(mtime_ns) = i64::try_from(mtime_ns) else { - return Ok(None); - }; - Ok(stored_sql_test_record(StoredSqlTestRecord { - identity, - relative_path, - size, - mtime_ns, - content_sha256, - })) -} - /// The valid cache records; a cache that cannot be read never fails compilation. pub(crate) fn read_cache( cache_dir: &Bound<'_, PyAny>, @@ -433,49 +379,8 @@ pub(crate) fn read_cache( if cache_dir.is_none() { return Ok(HashMap::new()); } - let cache_path: PathBuf = cache_dir.extract::()?.join(CACHE_FILE_NAME); - let Ok(metadata) = std::fs::metadata(&cache_path) else { - return Ok(HashMap::new()); - }; - if !metadata.is_file() || metadata.len() > MAX_CACHE_BYTES { - return Ok(HashMap::new()); - } - let Ok(contents) = std::fs::read(&cache_path) else { - return Ok(HashMap::new()); - }; - let Ok(text) = String::from_utf8(contents) else { - return Ok(HashMap::new()); - }; - let payload: Bound<'_, PyAny> = match imported(py, "json", "loads")?.call1((text,)) { - Ok(payload) => payload, - Err(error) if error.is_instance_of::(py) => { - return Ok(HashMap::new()); - } - Err(error) => return Err(error), - }; - let Ok(payload) = payload.downcast::() else { - return Ok(HashMap::new()); - }; - let version: Option> = payload.get_item("version")?; - if !version.is_some_and(|version| version.eq(CACHE_VERSION).unwrap_or(false)) { - return Ok(HashMap::new()); - } - let Some(raw_records) = payload.get_item("records")? else { - return Ok(HashMap::new()); - }; - let Ok(raw_records) = raw_records.downcast::() else { - return Ok(HashMap::new()); - }; - if raw_records.len() > MAX_CACHE_RECORDS { - return Ok(HashMap::new()); - } - let mut records: HashMap = HashMap::new(); - for (key, value) in raw_records.iter() { - if let Some(record) = stored_record(&value)? { - records.insert(key.extract()?, record); - } - } - Ok(records) + let directory: PathBuf = cache_dir.extract()?; + Ok(py.detach(|| read_sql_test_cache(&directory))) } /// Atomically replace the cache snapshot; a cache that cannot be written is ignored. @@ -487,44 +392,7 @@ pub(crate) fn write_cache( if cache_dir.is_none() { return Ok(()); } - let rows: Bound<'_, PyDict> = PyDict::new(py); - for (key, record) in records { - rows.set_item( - key, - dict( - py, - vec![ - ("identity", record.identity.as_str().into_bound_py_any(py)?), - ( - "relative_path", - record.relative_posix().into_bound_py_any(py)?, - ), - ("size", record.size.into_bound_py_any(py)?), - ("mtime_ns", record.mtime_ns.into_bound_py_any(py)?), - ( - "content_sha256", - record.content_sha256.as_str().into_bound_py_any(py)?, - ), - ], - )?, - )?; - } - let payload: Bound<'_, PyDict> = dict( - py, - vec![ - ("version", CACHE_VERSION.into_bound_py_any(py)?), - ("records", rows.into_any()), - ], - )?; - let encoded: String = compact_sorted_json(payload.as_any())?; let directory: PathBuf = cache_dir.extract()?; - let temporary: PathBuf = - directory.join(format!(".{CACHE_FILE_NAME}.{}.tmp", std::process::id())); - let written: std::io::Result<()> = py.detach(|| { - std::fs::create_dir_all(&directory)?; - std::fs::write(&temporary, encoded.as_bytes())?; - std::fs::rename(&temporary, directory.join(CACHE_FILE_NAME)) - }); - let _ = written; + py.detach(|| write_sql_test_cache(&directory, records)); Ok(()) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/json_report.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/json_report.rs index d2c57c1434..d729c5cc00 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/json_report.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/json_report.rs @@ -120,22 +120,6 @@ pub(crate) fn plain_json(value: &Bound<'_, PyAny>) -> PyResult { ) } -/// `json.dumps(value, ensure_ascii=True, separators=(",", ":"), sort_keys=True)`. -pub(crate) fn compact_sorted_json(value: &Bound<'_, PyAny>) -> PyResult { - let mut conversion: Conversion = Conversion::default(); - let converted: JsonValue = conversion.json_value(value)?; - if let Some(error) = conversion.stdlib_error { - return Err(error); - } - emitted( - &converted, - &JsonDialect::Stdlib(StdlibJsonOptions { - sort_keys: true, - ..StdlibJsonOptions::new(None).with_separators(",", ":") - }), - ) -} - fn emitted(value: &JsonValue, dialect: &JsonDialect) -> PyResult { dumps(value, dialect).map_err(|error| PyTypeError::new_err(format!("{error:?}"))) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/runtime_target.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/runtime_target.rs index 36d922bc4a..304afb25e6 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/runtime_target.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/runtime_target.rs @@ -13,7 +13,9 @@ use sqlbuild_analysis::target_paths::main::runtime_sql_text::runtime_sql_text; use sqlbuild_analysis::target_paths::main::sql_test_output_path::sql_test_output_path; use sqlbuild_analysis::target_paths::models::SqlTestPathFacts; use sqlbuild_core::text::main::python_isspace::python_isspace; -use sqlbuild_host::models::{BuildExecutionResult, LifeCycleEvent, LifeCycleEventKind}; +use sqlbuild_host::models::{ + BuildExecutionResult, LifeCycleEvent, LifeCycleEventKind, SqlTestExecutionResult, +}; use crate::bindings::_helpers::analysis::sql_test_comparisons::single_comparison_sql; use crate::bindings::_helpers::boundary::python_calls::imported; @@ -174,11 +176,19 @@ pub(crate) fn write_runtime_target( pub(crate) fn write_test_runtime_target( (target_dir, adapter): (&Bound<'_, PyAny>, &Bound<'_, PyAny>), plan: &Bound<'_, PyAny>, - results: &Bound<'_, PyAny>, + results: &[SqlTestExecutionResult>], ) -> PyResult<()> { let mut executed: HashSet = HashSet::new(); - for result in results.try_iter()? { - executed.insert(test_key(&result?, "test_name")?); + for result in results { + executed.insert(TestKey { + name: result.test_name.clone(), + source_path: result.source_path.as_ref().map(|path| { + path.to_string_lossy() + .replace(std::path::MAIN_SEPARATOR, "/") + }), + block_index: result.block_index, + case_name: result.case_name.clone(), + }); } let tests_dir: Bound<'_, PyAny> = target_dir.div(RUN_DIR)?.div(TESTS_DIR)?; for entry in native_field(plan, "test_entries")?.try_iter()? { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs b/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs index 533ecb2537..5e297e4b03 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs @@ -552,9 +552,17 @@ pub(crate) fn format_test_execution_json<'py>( py: Python<'py>, results: Bound<'py, PyAny>, ) -> PyResult { - let projection: Projection<'py> = Projection::installed(py)?; let all = Vec::>::read(&results)?; - let results = projection.retained(&all)?; + test_execution_json(py, &all) +} + +/// The test command's document from the executor's already-converted results. +pub(crate) fn test_execution_json<'py>( + py: Python<'py>, + all: &[SqlTestExecutionResult>], +) -> PyResult { + let projection: Projection<'py> = Projection::installed(py)?; + let results = projection.retained(all)?; let pass_count = results .iter() .filter(|result| result.outcome == SqlTestOutcome::Pass) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/sql_test_command/test_command.rs b/crates/sqlbuild-python/src/bindings/_helpers/sql_test_command/test_command.rs index aff77775db..777750ca28 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/sql_test_command/test_command.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/sql_test_command/test_command.rs @@ -19,6 +19,7 @@ use sqlbuild_core::test_command::main::test_name_width::test_name_width; use sqlbuild_core::test_command::main::test_outcome_status::test_outcome_status; use sqlbuild_core::test_command::models::{TestFailure, TestInspectionEntry, TestPlanWarning}; use sqlbuild_host::entries::models::ExecutorHost; +use sqlbuild_host::models::{SqlTestExecutionResult, SqlTestOutcome}; use crate::bindings::_helpers::boundary::python_calls::keywords; use crate::bindings::_helpers::boundary::python_calls::{KeywordArguments, call_with}; @@ -28,7 +29,7 @@ use crate::bindings::_helpers::build_progress::progress_reporters::ConnectionPro use crate::bindings::_helpers::build_progress::progress_text::{ format_expectation_detail, format_expectation_name, write_execution_header, }; -use crate::bindings::_helpers::build_progress::result_text::test_label; +use crate::bindings::_helpers::build_progress::result_text::{test_label, test_result_label}; use crate::bindings::_helpers::build_progress::transient_status::status_reporter; use crate::bindings::_helpers::change_selection::base_snapshot::{ BaseComparison, base_comparison, base_variant, @@ -43,13 +44,14 @@ use crate::bindings::_helpers::compile_driver::static_context::compile_static_co use crate::bindings::_helpers::compile_outputs::runtime_target::write_test_runtime_target; use crate::bindings::_helpers::dbt_interop::reference_resolver::resolve_external_sql_reference_resolver; use crate::bindings::_helpers::native_host::host; +use crate::bindings::_helpers::native_host::results::ReadResult; use crate::bindings::_helpers::planner_entries::command_plans::build_test_command_plan; use crate::bindings::_helpers::records::construct::error_class; use crate::bindings::_helpers::records::construct::{new_record_with, record_class}; use crate::bindings::_helpers::records::fields::field as native_field; use crate::bindings::_helpers::records::fields::replaced; use crate::bindings::_helpers::records::ids::{ErrorId, RecordId}; -use crate::bindings::_helpers::result_output::documents::format_test_execution_json; +use crate::bindings::_helpers::result_output::documents::test_execution_json; use crate::bindings::_helpers::result_output::documents::write_execution_json_output; use crate::bindings::_helpers::result_output::event_writer::ExecutionEventWriter; use crate::bindings::_helpers::result_output::resource_ids::sql_test_identity; @@ -57,8 +59,6 @@ use crate::bindings::_helpers::sql_test_command::timeouts::{ RunTimeout, TimeoutKind, resolve_run_timeout, }; -const PASS_OUTCOME: &str = "pass"; - /// Why `--since` selected each test, by inspection label. type SelectionReasons = HashMap>; /// What a `--since` selection kept: the narrowed pipeline, reasons by label, and its JSON section. @@ -482,7 +482,7 @@ fn run_tests<'py>( invocation: &Bound<'py, PyAny>, pipeline: &Bound<'py, PyAny>, (concurrency, timeout): (i64, &RunTimeout), -) -> PyResult>> { +) -> PyResult>>> { let py: Python<'py> = invocation.py(); let stream: Bound<'py, PyAny> = invocation.getattr("progress_stream")?; let use_color: Bound<'py, PyAny> = invocation.getattr("use_color")?; @@ -561,7 +561,7 @@ fn run_tests<'py>( connection: connection.clone().unbind(), }, )?; - let executed: PyResult>> = (|| { + let executed: PyResult>>> = (|| { let callbacks: Bound<'py, PyAny> = call_with( &host::entry(py, ExecutorHost::TestPipelineCallbacks)?, &[ @@ -598,10 +598,11 @@ fn run_tests<'py>( ], )? .try_iter()? + .map(|result| SqlTestExecutionResult::read(&result?)) .collect() })(); let closed: PyResult> = event_writer.call_method0("close"); - let results: Vec> = executed?; + let results: Vec>> = executed?; closed?; Ok(results) } @@ -618,21 +619,19 @@ pub(crate) fn timeout_record<'py>( fn complete( request: &Bound<'_, PyAny>, invocation: &Bound<'_, PyAny>, - (results, since_json): (&[Bound<'_, PyAny>], Option<&str>), + (results, since_json): (&[SqlTestExecutionResult>], Option<&str>), ) -> PyResult { let py: Python<'_> = request.py(); let mut failures: Vec = Vec::new(); for result in results { - if native_field(&native_field(result, "outcome")?, "value")?.extract::()? - == PASS_OUTCOME - { + if result.outcome == SqlTestOutcome::Pass { continue; } failures.push(TestFailure { - label: test_label(result, "test_name")?, - error_message: result.getattr("error_message")?.extract()?, - error_code: result.getattr("error_code")?.extract()?, - error_help: result.getattr("error_help")?.extract()?, + label: test_result_label(result)?, + error_message: result.error_message.clone(), + error_code: result.error_code.clone(), + error_help: result.error_help.clone(), }); } let passed: usize = results.len() - failures.len(); @@ -649,10 +648,9 @@ fn complete( ),), )?; stream.call_method0("flush")?; - let tuple: Bound<'_, PyAny> = PyTuple::new(py, results)?.into_any(); let payload: Bound<'_, PyAny> = PyString::new( py, - &with_since_section(format_test_execution_json(py, tuple)?, since_json)?, + &with_since_section(test_execution_json(py, results)?, since_json)?, ) .into_any(); write_execution_json_output( @@ -730,7 +728,7 @@ pub(crate) fn run_test_command(py: Python<'_>, request: &Bound<'_, PyAny>) -> Py request.getattr("test_timeout")?.extract()?, TimeoutKind::Test, )?; - let results: Vec> = + let results: Vec>> = run_tests(&invocation, &pipeline, (concurrency, &timeout))?; write_test_runtime_target( ( @@ -739,7 +737,7 @@ pub(crate) fn run_test_command(py: Python<'_>, request: &Bound<'_, PyAny>) -> Py &invocation.getattr("adapter")?, ), &pipeline.getattr("plan_output")?, - &PyTuple::new(py, &results)?.into_any(), + &results, )?; complete(request, &invocation, (&results, since_json.as_deref())) } diff --git a/fensu.toml b/fensu.toml index 8b03b9572a..c7a3c58a1e 100644 --- a/fensu.toml +++ b/fensu.toml @@ -1741,7 +1741,7 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/compile_command/startup_inputs.rs=4", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/surrogates.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs=23", - "crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs=63", + "crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs=58", "crates/sqlbuild-python/src/bindings/_helpers/compile_context/compile_timings.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/compile_context/diagnostic_collector.rs=10", "crates/sqlbuild-python/src/bindings/_helpers/compile_context/input_reads.rs=3", @@ -2225,7 +2225,7 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/semantic_shapes/coverage.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/semantic_shapes/shapes.rs=3", "crates/sqlbuild-python/src/bindings/_helpers/semantic_shapes/source_bindings.rs=13", - "crates/sqlbuild-python/src/bindings/_helpers/sql_test_command/test_command.rs=104", + "crates/sqlbuild-python/src/bindings/_helpers/sql_test_command/test_command.rs=101", "crates/sqlbuild-python/src/bindings/_helpers/sql_test_glue/assembly.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/sql_test_glue/authored_ctes.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/sql_test_glue/compiled_tests.rs=4", @@ -2303,7 +2303,6 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/compile_command/report.rs=6", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs=7", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs=4", - "crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/compile_context/diagnostic_collector.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/compile_context/input_reads.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/compile_driver/analysis.rs=12", diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_compile.py b/tests/integration/src/sqlbuild/cli/commands/main/test_compile.py index 1179b5175b..488e0d48d0 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_compile.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_compile.py @@ -382,7 +382,8 @@ def test_given_sql_test_chain_when_compiling_then_native_planner_writes_complete encoding="utf-8", ) - exit_code: int = main(["--project-dir", str(tmp_path), "compile", "--json", "--no-cache"]) + arguments: list[str] = ["--project-dir", str(tmp_path), "compile", "--json"] + exit_code: int = main(arguments) result: dict[str, object] = json.loads(capsys.readouterr().out) artifact_path: Path = ( tmp_path @@ -409,6 +410,37 @@ def test_given_sql_test_chain_when_compiling_then_native_planner_writes_complete assert "SELECT 2 AS order_id, 20 AS amount" in second_artifact_sql assert "SELECT 1 AS order_id, 10 AS amount" not in second_artifact_sql + cache_path: Path = ( + tmp_path / "target" / "cache" / "compiler-native-v1" / "sql-test-artifacts.json" + ) + snapshot_text: str = cache_path.read_text(encoding="utf-8") + snapshot: dict = json.loads(snapshot_text) + assert snapshot["version"] == 5 + assert len(snapshot["records"]) == test_case.expected_count + assert snapshot_text == json.dumps(snapshot, sort_keys=True, separators=(",", ":")) + + config: Path = tmp_path / "sqlbuild_project.toml" + original_config: str = config.read_text(encoding="utf-8") + config.write_text(original_config + '\n[vars]\nrevision = "1"\n', encoding="utf-8") + snapshot["version"] = 5.0 + cache_path.write_text(json.dumps(snapshot), encoding="utf-8") + assert main(arguments) == 0, capsys.readouterr() + _ = capsys.readouterr() + assert json.loads(cache_path.read_text(encoding="utf-8"))["records"] == snapshot["records"] + assert artifact_path.read_text(encoding="utf-8") == artifact_sql + + broken_snapshot: dict = json.loads(cache_path.read_text(encoding="utf-8")) + next(iter(broken_snapshot["records"].values()))["size"] = True + cache_path.write_text(json.dumps(broken_snapshot), encoding="utf-8") + config.write_text(original_config + '\n[vars]\nrevision = "2"\n', encoding="utf-8") + assert main(arguments) == 0, capsys.readouterr() + _ = capsys.readouterr() + assert json.loads(cache_path.read_text(encoding="utf-8"))["records"] == snapshot["records"] + assert artifact_path.read_text(encoding="utf-8") == artifact_sql + assert artifact_path.with_name("orders_chain_second.sql").read_text(encoding="utf-8") == ( + second_artifact_sql + ) + if __name__ == "__main__": pytest.main([__file__, "-vv"]) From 81f2dccfef65f465ffa506b5625a45eff1eeebd5 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:58:02 +0000 Subject: [PATCH 2/8] refactor: keep compile reuse inputs native Construct reuse checks and compile records directly in Rust instead of round-tripping through Python containers. Reuse the cache artifact model while preserving Python-facing rows and launch-record strings. Validated with 65 focused unit and integration tests, cold/warm native launcher replay, and make check-ci. Existing launcher coverage caught and now guards the path serialization conversion. Amp-Thread-ID: https://ampcode.com/threads/T-01a18715-4e00-708e-a87d-6bd109748d66 --- .../_helpers/compile_command/reuse_attempt.rs | 173 +++++++----------- .../compile_command/reuse_recording.rs | 167 ++++++++--------- .../compile_command/runtime_identity.rs | 33 ++-- .../_helpers/compile_outputs/reuse.rs | 106 +++++------ fensu.toml | 4 +- 5 files changed, 216 insertions(+), 267 deletions(-) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_attempt.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_attempt.rs index 9d6db0071b..77c109624c 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_attempt.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_attempt.rs @@ -7,8 +7,9 @@ use pyo3::prelude::{ Bound, PyAny, PyAnyMethods, PyErr, PyModule, PyModuleMethods, PyResult, Python, }; use pyo3::types::{PyDict, PyDictMethods, PyFrozenSet, PyList, PyString}; -use pyo3::{IntoPyObjectExt, pyfunction, wrap_pyfunction}; +use pyo3::{IntoPyObject, IntoPyObjectExt, pyfunction, wrap_pyfunction}; use sqlbuild_cache::project_reuse::constants::FAILED_COMPILED_DIRECTORY_NAME; +use sqlbuild_cli::replay::main::output_file_identities::output_file_identities; use sqlbuild_cli::replay::main::write_launch_record::write_launch_record as write_record_file; use sqlbuild_core::command_output::constants::INVOCATION_CONTEXT_ENV; @@ -34,7 +35,7 @@ use crate::bindings::_helpers::compile_command::constants::{ }; use crate::bindings::_helpers::compile_command::models::CompileReuseAttempt; use crate::bindings::_helpers::compile_command::runtime_identity::{ - invocation_identity, redirected_output_files, runtime_identity, + invocation_identity, runtime_identity, }; use crate::bindings::_helpers::compile_context::delta; use crate::bindings::_helpers::compile_delta::driver::{ @@ -44,7 +45,9 @@ use crate::bindings::_helpers::compile_delta::verify; use crate::bindings::_helpers::compile_driver::command::run_compile_command; use crate::bindings::_helpers::compile_driver::phases::{elapsed_ms, monotonic}; use crate::bindings::_helpers::compile_outputs::json_report::plain_json; -use crate::bindings::_helpers::compile_outputs::reuse::{ReuseCheckPy, check_compile_reuse}; +use crate::bindings::_helpers::compile_outputs::reuse::{ + ReuseCheckPy, ReuseRulesPy, check_compile_reuse, +}; use crate::bindings::_helpers::frontier::cache_directory::cache_directory; use crate::bindings::_helpers::macro_bridge::first_compile::keep_delta_claim; use crate::bindings::_helpers::native_host::host; @@ -62,10 +65,6 @@ const MISS: &str = "miss"; const LOG_TEXT: &str = "%s"; const BYPASS: &str = "bypass"; -fn listed<'py>(value: Bound<'py, PyAny>) -> PyResult> { - Ok(PyList::new(value.py(), value.try_iter()?.collect::>>()?)?.into_any()) -} - fn reuse_logger(py: Python<'_>) -> PyResult> { imported(py, "logging", "getLogger")?.call1((REUSE_LOGGER_NAME,)) } @@ -86,78 +85,41 @@ fn reuse_debug_with<'py>( } /// The reuse constants the native check, the stored compile and the launch record share. -pub(crate) fn reuse_rules(py: Python<'_>, with_output_files: bool) -> PyResult> { - let rules: Bound<'_, PyDict> = PyDict::new(py); - rules.set_item("excluded", PyList::new(py, EXCLUDED_DIRECTORIES)?)?; - rules.set_item( - "excluded_root", - imported(py, "builtins", "sorted")?.call1((registry::native( - py, - NativeAttr::CompileReuseExcludedRootDirectories, - )?,))?, - )?; - rules.set_item( - "presence_suffixes", - PyList::new(py, PRESENCE_ONLY_FILE_SUFFIXES)?, - )?; - let output_files: Bound<'_, PyList> = redirected_output_files(py)?; - if with_output_files { - rules.set_item("output_files", output_files)?; - } - rules.set_item( - "tracked_prefixes", - PyList::new(py, TRACKED_ENVIRONMENT_PREFIXES)?, - )?; - rules.set_item( - "untracked_names", - PyList::new(py, UNTRACKED_ENVIRONMENT_NAMES)?, - )?; - rules.set_item( - "disabling_variables", - PyList::new( - py, - [ - ( - COMPILE_CACHE_DISABLE_ENV_VAR.into_bound_py_any(py)?, - COMPILE_CACHE_DISABLE_VALUE.into_bound_py_any(py)?, - ), - ( - REUSE_DISABLE_ENV_VAR.into_bound_py_any(py)?, - REUSE_DISABLE_VALUE.into_bound_py_any(py)?, - ), - ], - )?, - )?; - rules.set_item( - "config_filenames", - PyList::new(py, PROJECT_CONFIG_FILENAMES)?, - )?; - for (key, value) in [ - ("max_stored_entries", REUSE_MAX_STORED_ENTRIES), - ("max_entry_bytes", REUSE_MAX_ENTRY_BYTES), - ("missing_path_mtime_ns", MISSING_PATH_MTIME_NS), - ("project_root_path_mtime_ns", PROJECT_ROOT_PATH_MTIME_NS), - ] { - rules.set_item(key, value)?; - } - rules.set_item( - "racy_window_ns", - registry::native(py, NativeAttr::CompileReuseRacyWindowNs)?, - )?; - rules.set_item("missing_environment_value", MISSING_ENVIRONMENT_VALUE)?; - rules.set_item("missing_file_digest", MISSING_FILE_DIGEST)?; - rules.set_item( - "compiled_root", - format!( - "{TARGET_DIRECTORY_NAME}{}{COMPILED_DIRECTORY_NAME}", - std::path::MAIN_SEPARATOR - ), - )?; - rules.set_item( - "retired_directories", - PyList::new(py, RETIRED_REUSE_DIRECTORY_NAMES)?, - )?; - Ok(rules) +pub(crate) fn reuse_rules(py: Python<'_>) -> PyResult { + let mut excluded_root: Vec = + registry::native(py, NativeAttr::CompileReuseExcludedRootDirectories)? + .try_iter()? + .map(|value| value?.extract()) + .collect::>()?; + excluded_root.sort(); + Ok(ReuseRulesPy { + excluded: EXCLUDED_DIRECTORIES.map(str::to_owned).into(), + excluded_root, + presence_suffixes: PRESENCE_ONLY_FILE_SUFFIXES.map(str::to_owned).into(), + output_files: output_file_identities(), + tracked_prefixes: TRACKED_ENVIRONMENT_PREFIXES.map(str::to_owned).into(), + untracked_names: UNTRACKED_ENVIRONMENT_NAMES.map(str::to_owned).into(), + disabling_variables: vec![ + ( + COMPILE_CACHE_DISABLE_ENV_VAR.to_owned(), + COMPILE_CACHE_DISABLE_VALUE.to_owned(), + ), + ( + REUSE_DISABLE_ENV_VAR.to_owned(), + REUSE_DISABLE_VALUE.to_owned(), + ), + ], + config_filenames: PROJECT_CONFIG_FILENAMES.map(str::to_owned).into(), + racy_window_ns: registry::native(py, NativeAttr::CompileReuseRacyWindowNs)?.extract()?, + max_stored_entries: REUSE_MAX_STORED_ENTRIES as usize, + max_entry_bytes: REUSE_MAX_ENTRY_BYTES as usize, + missing_environment_value: MISSING_ENVIRONMENT_VALUE.to_owned(), + missing_file_digest: MISSING_FILE_DIGEST.to_owned(), + missing_path_mtime_ns: MISSING_PATH_MTIME_NS, + project_root_path_mtime_ns: PROJECT_ROOT_PATH_MTIME_NS, + compiled_root: PathBuf::from(TARGET_DIRECTORY_NAME).join(COMPILED_DIRECTORY_NAME), + retired_directories: RETIRED_REUSE_DIRECTORY_NAMES.map(str::to_owned).into(), + }) } /// The launch record `sqb` replays this reusable compile from, as a JSON object. @@ -214,7 +176,12 @@ fn launch_payload<'py>( host::entry(py, FacadeHost::StartupInputs)?.call0()?, )?; payload.set_item("json_output", native_field(attempt, "json_output")?)?; - payload.set_item("rules", reuse_rules(py, false)?)?; + let native_rules: ReuseRulesPy = reuse_rules(py)?; + let compiled_root = native_rules.compiled_root.clone().into_os_string(); + let rules: Bound<'py, PyDict> = native_rules.into_pyobject(py)?; + rules.set_item("compiled_root", compiled_root)?; + rules.del_item("output_files")?; + payload.set_item("rules", rules)?; payload.set_item("capture", capture)?; payload.set_item("hit_message", PyList::new(py, REUSE_HIT_MESSAGE)?)?; payload.set_item("timings", timings)?; @@ -286,31 +253,26 @@ fn reuse_attempt<'py>( fn reuse_check<'py>( request: &Bound<'py, PyAny>, project_dir: &Bound<'py, PyAny>, -) -> PyResult> { +) -> PyResult { let py: Python<'py> = request.py(); let flag = |name: &str| -> PyResult { native_field(request, name)?.is_truthy() }; let use_color: bool = !flag("no_color")? && stdout_supports_color(py)?; - let check: Bound<'py, PyDict> = PyDict::new(py); - check.set_item("project_dir", project_dir)?; - check.set_item( - "store_directory", - cache_directory(&imported(py, "pathlib", "Path")?.call1((project_dir,))?)? - .call_method1("__truediv__", (NATIVE_REUSE_DIRECTORY_NAME,))?, - )?; - check.set_item("selected_target", native_field(request, "selected_target")?)?; - check.set_item( - "invocation", - invocation_identity(request, project_dir, use_color)?, - )?; - check.set_item("runtime", runtime_identity(py)?)?; - check.set_item("search_path", listed(py.import("sys")?.getattr("path")?)?)?; - check.set_item( - "bypass_requested", - flag("no_cache")? || flag("manifest")? || flag("profiling")? || flag("debug")?, - )?; - check.set_item("json_output", native_field(request, "json_output")?)?; - check.set_item("rules", reuse_rules(py, true)?)?; - Ok(check) + let directory: PathBuf = + cache_directory(&imported(py, "pathlib", "Path")?.call1((project_dir,))?)?.extract()?; + Ok(ReuseCheckPy { + project_dir: project_dir.extract()?, + store_directory: directory.join(NATIVE_REUSE_DIRECTORY_NAME), + selected_target: native_field(request, "selected_target")?.extract()?, + invocation: invocation_identity(request, project_dir, use_color)?.extract()?, + runtime: runtime_identity(py)?, + search_path: py.import("sys")?.getattr("path")?.extract()?, + bypass_requested: flag("no_cache")? + || flag("manifest")? + || flag("profiling")? + || flag("debug")?, + json_output: native_field(request, "json_output")?.extract()?, + rules: reuse_rules(py)?, + }) } /// `print(text, file=sys.stderr)`. @@ -409,11 +371,8 @@ fn attempt_compile_reuse<'py>(request: &Bound<'py, PyAny>) -> PyResult = check_compile_reuse( - py, - reuse_check(request, &project_dir)?.extract::()?, - ) - .and_then(|value| pyo3::IntoPyObjectExt::into_bound_py_any(value, py))?; + let native: Bound<'py, PyAny> = check_compile_reuse(py, reuse_check(request, &project_dir)?) + .and_then(|value| pyo3::IntoPyObjectExt::into_bound_py_any(value, py))?; let checked: String = native.call_method0("outcome")?.extract()?; let json_output: Bound<'py, PyAny> = native_field(request, "json_output")?; let mut base: Vec<(&'static str, Bound<'py, PyAny>)> = vec![ diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs index 400b3c3faf..07f03ea95b 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs @@ -6,10 +6,13 @@ use std::path::{Path, PathBuf}; use std::sync::{Mutex, PoisonError}; use pyo3::exceptions::{PyException, PyOSError}; -use pyo3::prelude::{Bound, Py, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyResult, Python}; +use pyo3::prelude::{ + Bound, Py, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyRef, PyResult, Python, +}; use pyo3::types::{PyDict, PyDictMethods, PyList, PyListMethods, PyString, PyStringMethods}; -use pyo3::{IntoPyObjectExt, pyclass, pyfunction, pymethods, wrap_pyfunction}; +use pyo3::{IntoPyObject, IntoPyObjectExt, pyclass, pyfunction, pymethods, wrap_pyfunction}; +use sqlbuild_cache::project_reuse::models::RecordedArtifact; use sqlbuild_core::posix_path::main::posix_abspath::posix_abspath; use sqlbuild_host::entries::models::{FacadeHost, UserCodeHost}; @@ -22,7 +25,9 @@ use crate::bindings::_helpers::compile_command::constants::{ REUSE_STORE_NOTICE_PATHS, REUSE_STORE_SKIPPED_PREFIX, }; use crate::bindings::_helpers::compile_command::reuse_attempt::{reuse_rules, write_launch_record}; -use crate::bindings::_helpers::compile_outputs::reuse::artifact_digest; +use crate::bindings::_helpers::compile_outputs::reuse::{ + ArtifactRow, CompileRecordPy, NativeReuseAttempt, SettingsRow, artifact_digest, +}; use crate::bindings::_helpers::native_host::host; use crate::bindings::_helpers::records::fields::field as native_field; @@ -32,13 +37,6 @@ const BYPASS_OUTCOME: &str = "bypass"; const STORED: &str = "stored"; const SKIPPED: &str = "skipped"; -/// What a compile left at one artifact path: its digest, or the size and mtime it kept. -#[derive(Clone, Debug, PartialEq, Eq)] -enum RecordedArtifact { - Written(String), - Kept { size: u64, mtime_ns: i64 }, -} - /// Artifacts one compile wrote or kept, by absolute path, following staged moves. #[pyclass(module = "sqlbuild._native", frozen)] pub(crate) struct CompileArtifactWrites { @@ -88,13 +86,29 @@ impl CompileArtifactWrites { .iter() .map(|(path, artifact)| { let digest: Option = match artifact { - RecordedArtifact::Written(digest) => Some(digest.clone()), + RecordedArtifact::Written { digest } => Some(digest.clone()), RecordedArtifact::Kept { .. } => None, }; (path.clone(), digest) }) .collect() } + + /// The Python-facing artifact rows, built natively for recording or the public view. + fn artifact_rows(&self) -> HashMap { + self.locked() + .iter() + .map(|(path, artifact)| { + let row = match artifact { + RecordedArtifact::Written { digest } => (Some(digest.clone()), None, None), + RecordedArtifact::Kept { size, mtime_ns } => { + (None, Some(*size), Some(*mtime_ns)) + } + }; + (path.clone(), row) + }) + .collect() + } } #[pymethods] @@ -102,18 +116,7 @@ impl CompileArtifactWrites { /// Every recorded artifact by absolute path, as `(digest, size, mtime_ns)`. #[getter] pub(crate) fn artifacts<'py>(&self, py: Python<'py>) -> PyResult> { - let rows: Bound<'py, PyDict> = PyDict::new(py); - for (path, artifact) in self.locked().iter() { - match artifact { - RecordedArtifact::Written(digest) => { - rows.set_item(path, (digest, py.None(), py.None()))?; - } - RecordedArtifact::Kept { size, mtime_ns } => { - rows.set_item(path, (py.None(), size, mtime_ns))?; - } - } - } - Ok(rows) + self.artifact_rows().into_pyobject(py) } } @@ -165,7 +168,9 @@ pub(crate) fn artifact_written( }; recorder.get().record( absolute(path)?, - RecordedArtifact::Written(artifact_digest(contents)), + RecordedArtifact::Written { + digest: artifact_digest(contents), + }, ); Ok(()) } @@ -182,7 +187,9 @@ pub(crate) fn artifact_written_at( }; recorder.get().record( absolute_path(py, path)?, - RecordedArtifact::Written(artifact_digest(contents)), + RecordedArtifact::Written { + digest: artifact_digest(contents), + }, ); Ok(()) } @@ -455,75 +462,55 @@ fn record<'py>(facts: &CompileFacts<'py>) -> PyResult { } else { None }; - let settings_inputs: Bound<'_, PyAny> = if unsupported.is_none() { - let rows: Bound<'_, PyList> = PyList::empty(py); - let list = |value: Bound<'py, PyAny>| -> PyResult> { - Ok(PyList::new(py, value.try_iter()?.collect::>>()?)?.into_any()) - }; + let strings = |value: Bound<'py, PyAny>| -> PyResult> { + value.try_iter()?.map(|item| item?.extract()).collect() + }; + let settings_inputs: Option> = if unsupported.is_none() { + let mut rows: Vec = Vec::new(); for item in native_field(&settings, "inputs")?.try_iter()? { let item: Bound<'_, PyAny> = item?; - rows.append(( - native_field(&item, "case_sensitive")?, - list(native_field(&item, "names")?)?, - list(native_field(&item, "prefixes")?)?, - list(native_field(&item, "env_files")?)?, - list(native_field(&item, "secrets_dirs")?)?, - ))?; + rows.push(SettingsRow { + case_sensitive: native_field(&item, "case_sensitive")?.extract()?, + names: strings(native_field(&item, "names")?)?, + prefixes: strings(native_field(&item, "prefixes")?)?, + env_files: strings(native_field(&item, "env_files")?)?, + secrets_dirs: strings(native_field(&item, "secrets_dirs")?)?, + }); } - rows.into_any() + Some(rows) } else { - py.None().into_bound(py) + None }; - let dag_path: Bound<'_, PyAny> = if facts.dag_artifact_path.is_none() { - py.None().into_bound(py) + let dag_path: Option = if facts.dag_artifact_path.is_none() { + None } else { - imported(py, "os.path", "abspath")?.call1((&facts.dag_artifact_path,))? + Some( + imported(py, "os.path", "abspath")? + .call1((&facts.dag_artifact_path,))? + .extract()?, + ) + }; + let payload = CompileRecordPy { + stdout: stdout.extract()?, + stderr_lines: strings(native_field(&facts.output, "stderr_lines")?)?, + exit_code: facts.exit_code.extract()?, + compile_cache_enabled: facts.compile_cache_enabled, + read_run_id: read_run_id.extract()?, + settings_inputs, + template_environment_names: strings(native_field( + &facts.input_reads, + "environment_names", + )?)?, + module_paths: module_paths(py)?.extract()?, + artifacts: facts + .artifact_writes + .extract::>()? + .artifact_rows(), + artifacts_written: facts.artifacts_written, + dag_path, + json_output: facts.json_output.extract()?, + rules: reuse_rules(py)?, }; - let payload: Bound<'_, PyDict> = keywords( - py, - &[ - ("stdout", stdout), - ( - "stderr_lines", - PyList::new( - py, - native_field(&facts.output, "stderr_lines")? - .try_iter()? - .collect::>>()?, - )? - .into_any(), - ), - ("exit_code", facts.exit_code.clone()), - ( - "compile_cache_enabled", - facts.compile_cache_enabled.into_bound_py_any(py)?, - ), - ("read_run_id", read_run_id), - ("settings_inputs", settings_inputs), - ( - "template_environment_names", - PyList::new( - py, - native_field(&facts.input_reads, "environment_names")? - .try_iter()? - .collect::>>()?, - )? - .into_any(), - ), - ("module_paths", module_paths(py)?.into_any()), - ( - "artifacts", - native_field(&facts.artifact_writes, "artifacts")?, - ), - ( - "artifacts_written", - facts.artifacts_written.into_bound_py_any(py)?, - ), - ("dag_path", dag_path), - ("json_output", facts.json_output.clone()), - ("rules", reuse_rules(py, true)?.into_any()), - ], - )?; if let Some(mut capture) = crate::bindings::_helpers::compile_context::delta::take_capture() { capture.environment_names = native_field(&facts.input_reads, "environment_names")?.extract()?; @@ -547,8 +534,10 @@ fn record<'py>(facts: &CompileFacts<'py>) -> PyResult { } } } - let (outcome, digested): (String, Bound<'_, PyAny>) = - facts.native.call_method1("record", (payload,))?.extract()?; + let (outcome, digested): (&str, usize) = facts + .native + .extract::>()? + .record(py, payload)?; let _ = reuse_logger(py)?.call_method1( "debug", ("Compile reuse record digested %d project files", digested), diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/runtime_identity.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/runtime_identity.rs index af0aaa811d..0f9299991a 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/runtime_identity.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/runtime_identity.rs @@ -1,9 +1,11 @@ //! Interpreter, native build and invocation facts that decide whether a stored compile is reusable. +use std::collections::BTreeMap; +use std::ffi::OsString; + use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyResult, Python}; use pyo3::types::{PyDict, PyDictMethods, PyList, PyTuple}; use pyo3::{IntoPyObjectExt, pyfunction, wrap_pyfunction}; -use sqlbuild_cli::replay::main::output_file_identities::output_file_identities; use crate::bindings::_helpers::boundary::python_calls::keywords; use crate::bindings::_helpers::records::fields::field as native_field; @@ -46,19 +48,19 @@ fn interpreter_identity(py: Python<'_>) -> PyResult> { } /// The interpreter and native build identity that produced a compile. -pub(crate) fn runtime_identity(py: Python<'_>) -> PyResult> { +pub(crate) fn runtime_identity(py: Python<'_>) -> PyResult> { let [version, executable, platform, cache_tag] = interpreter_values(py)?; - let identity: Bound<'_, PyDict> = PyDict::new(py); - identity.set_item("format", REUSE_FORMAT_VERSION)?; - identity.set_item("python", version)?; - identity.set_item("executable", executable)?; - identity.set_item("platform", platform)?; - identity.set_item("cache_tag", cache_tag)?; - identity.set_item( - "native_build", - registry::native(py, NativeAttr::BuildIdentity)?.str()?, - )?; - Ok(identity) + let native_build: String = registry::native(py, NativeAttr::BuildIdentity)? + .str()? + .extract()?; + Ok(BTreeMap::from([ + ("format".to_owned(), REUSE_FORMAT_VERSION.into()), + ("python".to_owned(), version.into()), + ("executable".to_owned(), executable.into()), + ("platform".to_owned(), platform.into()), + ("cache_tag".to_owned(), cache_tag.into()), + ("native_build".to_owned(), native_build.into()), + ])) } fn listed<'py>(value: Bound<'py, PyAny>) -> PyResult> { @@ -104,11 +106,6 @@ pub(crate) fn invocation_identity<'py>( text.call_method1("encode", ("utf-8", "surrogateescape")) } -/// Device and inode pairs of stdout and stderr files, which hold output rather than inputs. -pub(crate) fn redirected_output_files(py: Python<'_>) -> PyResult> { - PyList::new(py, output_file_identities()) -} - pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { module.add_function(wrap_pyfunction!(interpreter_identity, module)?) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/reuse.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/reuse.rs index e07ab58f5f..4407d2e31d 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/reuse.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/reuse.rs @@ -7,7 +7,7 @@ use std::sync::Mutex; use pyo3::exceptions::PyValueError; use pyo3::prelude::{Bound, PyModule, PyModuleMethods, PyResult, Python}; -use pyo3::{FromPyObject, pyclass, pyfunction, pymethods, wrap_pyfunction}; +use pyo3::{FromPyObject, IntoPyObject, pyclass, pyfunction, pymethods, wrap_pyfunction}; use sqlbuild_cache::digest::main::bytes_digest::bytes_digest; use sqlbuild_cache::digest::main::hex_digest::hex_digest; use sqlbuild_cache::project_reuse::constants::{NATIVE_DIGEST_PREFIX, REUSE_STORE_SUFFIX}; @@ -27,40 +27,40 @@ use crate::bindings::_helpers::boundary::panics::compiler_guard; positional_record! { extract; /// Settings case sensitivity, names, prefixes, env files and secrets directories. - struct SettingsRow { - case_sensitive: bool, - names: Vec, - prefixes: Vec, - env_files: Vec, - secrets_dirs: Vec, + pub(crate) struct SettingsRow { + pub(crate) case_sensitive: bool, + pub(crate) names: Vec, + pub(crate) prefixes: Vec, + pub(crate) env_files: Vec, + pub(crate) secrets_dirs: Vec, } } type ReplayRow = (String, Vec, i64); type LaunchIdentityRow = (String, BTreeMap, Vec); /// A written artifact's digest, or a kept artifact's size and mtime. -type ArtifactRow = (Option, Option, Option); +pub(crate) type ArtifactRow = (Option, Option, Option); /// The host constants the walk, environment and store checks use. -#[derive(FromPyObject)] +#[derive(FromPyObject, IntoPyObject)] #[pyo3(from_item_all)] -struct ReuseRulesPy { - excluded: Vec, - excluded_root: Vec, - presence_suffixes: Vec, - output_files: Vec<(u64, u64)>, - tracked_prefixes: Vec, - untracked_names: Vec, - disabling_variables: Vec<(String, String)>, - config_filenames: Vec, - racy_window_ns: i64, - max_stored_entries: usize, - max_entry_bytes: usize, - missing_environment_value: String, - missing_file_digest: String, - missing_path_mtime_ns: i64, - project_root_path_mtime_ns: i64, - compiled_root: PathBuf, - retired_directories: Vec, +pub(crate) struct ReuseRulesPy { + pub(crate) excluded: Vec, + pub(crate) excluded_root: Vec, + pub(crate) presence_suffixes: Vec, + pub(crate) output_files: Vec<(u64, u64)>, + pub(crate) tracked_prefixes: Vec, + pub(crate) untracked_names: Vec, + pub(crate) disabling_variables: Vec<(String, String)>, + pub(crate) config_filenames: Vec, + pub(crate) racy_window_ns: i64, + pub(crate) max_stored_entries: usize, + pub(crate) max_entry_bytes: usize, + pub(crate) missing_environment_value: String, + pub(crate) missing_file_digest: String, + pub(crate) missing_path_mtime_ns: i64, + pub(crate) project_root_path_mtime_ns: i64, + pub(crate) compiled_root: PathBuf, + pub(crate) retired_directories: Vec, } impl From for ReuseRules { @@ -93,34 +93,34 @@ impl From for ReuseRules { #[derive(FromPyObject)] #[pyo3(from_item_all)] pub(crate) struct ReuseCheckPy { - project_dir: PathBuf, - store_directory: PathBuf, - selected_target: Option, - invocation: Vec, - runtime: BTreeMap, - search_path: Vec, - bypass_requested: bool, - json_output: bool, - rules: ReuseRulesPy, + pub(crate) project_dir: PathBuf, + pub(crate) store_directory: PathBuf, + pub(crate) selected_target: Option, + pub(crate) invocation: Vec, + pub(crate) runtime: BTreeMap, + pub(crate) search_path: Vec, + pub(crate) bypass_requested: bool, + pub(crate) json_output: bool, + pub(crate) rules: ReuseRulesPy, } /// What a full compile leaves for recording; `artifacts` maps a path to `(digest, size, mtime)`. #[derive(FromPyObject)] #[pyo3(from_item_all)] -struct CompileRecordPy { - stdout: Option, - stderr_lines: Vec, - exit_code: i64, - compile_cache_enabled: bool, - read_run_id: bool, - settings_inputs: Option>, - template_environment_names: Vec, - module_paths: Vec, - artifacts: HashMap, - artifacts_written: bool, - dag_path: Option, - json_output: bool, - rules: ReuseRulesPy, +pub(crate) struct CompileRecordPy { + pub(crate) stdout: Option, + pub(crate) stderr_lines: Vec, + pub(crate) exit_code: i64, + pub(crate) compile_cache_enabled: bool, + pub(crate) read_run_id: bool, + pub(crate) settings_inputs: Option>, + pub(crate) template_environment_names: Vec, + pub(crate) module_paths: Vec, + pub(crate) artifacts: HashMap, + pub(crate) artifacts_written: bool, + pub(crate) dag_path: Option, + pub(crate) json_output: bool, + pub(crate) rules: ReuseRulesPy, } /// One invocation's native reuse state, from the check to the record. @@ -202,7 +202,11 @@ impl NativeReuseAttempt { } /// Store this compile or clear the slot; a failure clears the slot and raises `OSError`. - fn record(&self, py: Python<'_>, record: CompileRecordPy) -> PyResult<(&'static str, usize)> { + pub(crate) fn record( + &self, + py: Python<'_>, + record: CompileRecordPy, + ) -> PyResult<(&'static str, usize)> { compiler_guard(|| { let rules: ReuseRules = record.rules.into(); let mut compile: CompileRecord = CompileRecord { diff --git a/fensu.toml b/fensu.toml index c7a3c58a1e..b933e33f59 100644 --- a/fensu.toml +++ b/fensu.toml @@ -1735,8 +1735,8 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/compile_command/models.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/prepared_artifacts.rs=42", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/report.rs=30", - "crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_attempt.rs=48", - "crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs=41", + "crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_attempt.rs=45", + "crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs=40", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/runtime_identity.rs=14", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/startup_inputs.rs=4", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/surrogates.rs=2", From 2dcbd56b7da6b6085eb3c9e6434486082782628d Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 30 Oct 2026 00:17:33 +0000 Subject: [PATCH 3/8] refactor: retain native prepared SQL-test artifact plans Keep SQL-test plans, cached paths and managed paths native through preparation and publication. Preserve Python plan projection at the public API and platform path equality at target matching. Amp-Thread-ID: https://ampcode.com/threads/T-01a18715-4e00-708e-a87d-6bd109748d66 --- .../compile_command/prepared_artifacts.rs | 23 +-- .../compile_command/reuse_recording.rs | 10 +- .../_helpers/compile_command/target_writer.rs | 161 ++++++------------ .../compile_outputs/runtime_target.rs | 17 +- .../src/bindings/_helpers/records/ids.rs | 2 - .../_helpers/refactor/offline_compile.rs | 25 +-- .../_helpers/sql_test_glue/plan_driver.rs | 48 ++++-- fensu.toml | 2 +- 8 files changed, 117 insertions(+), 171 deletions(-) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/prepared_artifacts.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/prepared_artifacts.rs index de793bf5cc..689548f6fd 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/prepared_artifacts.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/prepared_artifacts.rs @@ -12,7 +12,7 @@ use crate::bindings::_helpers::boundary::python_calls::{ }; use crate::bindings::_helpers::compile_command::reuse_recording::CompileArtifactWrites; use crate::bindings::_helpers::compile_command::target_writer::{ - PlannedStaticSqlTests, plan_static_sql_tests, + PlannedStaticSqlTests, plan_tests, }; use crate::bindings::_helpers::compile_command::target_writer::{ Prepared, planning_diagnostics, publish_staged_target, staging_changed, write_static_target, @@ -293,23 +293,10 @@ impl PreparedCompileArtifacts { -> PyResult> { let py: Python<'_> = arguments.py(); count_stage(py, BACKGROUND_SQL_TEST_PLANS, 1)?; - let planned: PlannedStaticSqlTests = plan_static_sql_tests( - py, - Some(keywords( - py, - &[ - ("target_dir", planned_target.bind(py).clone()), - ("adapter", planned_adapter.bind(py).clone()), - ("project", planned_project.bind(py).clone()), - ( - "artifact_writes", - recorder.as_ref().map_or_else( - || py.None().into_bound(py), - |recorder| recorder.bind(py).clone().into_any(), - ), - ), - ], - )?), + let planned: PlannedStaticSqlTests = plan_tests( + planned_target.bind(py), + (planned_adapter.bind(py), planned_project.bind(py)), + recorder.as_ref(), )?; Ok(Py::new(py, planned)?.into_any()) }; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs index 07f03ea95b..47686ac9fa 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs @@ -196,16 +196,18 @@ pub(crate) fn artifact_written_at( /// Note an earlier artifact kept as-is after its size and mtime were validated. pub(crate) fn artifact_kept( + py: Python<'_>, recorder: ArtifactRecorder<'_>, - path: &Bound<'_, PyAny>, + path: &Path, (size, mtime_ns): (u64, i64), ) -> PyResult<()> { let Some(recorder) = recorder else { return Ok(()); }; - recorder - .get() - .record(absolute(path)?, RecordedArtifact::Kept { size, mtime_ns }); + recorder.get().record( + absolute_path(py, path)?, + RecordedArtifact::Kept { size, mtime_ns }, + ); Ok(()) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs index 7432a8e859..4bc7b64b92 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs @@ -23,7 +23,7 @@ use crate::bindings::_helpers::boundary::python_calls::{ use crate::bindings::_helpers::compile_command::artifact_batch::ArtifactBatch; use crate::bindings::_helpers::compile_command::constants::RACY_WINDOW_NS; use crate::bindings::_helpers::compile_command::reuse_recording::{ - ArtifactRecorder, CompileArtifactWrites, artifact_kept, artifact_moved, artifact_tree_moved, + ArtifactRecorder, artifact_kept, artifact_moved, artifact_tree_moved, }; use crate::bindings::_helpers::compile_command::test_artifact_cache::{ IdentityContext, artifact_identity_payload, artifact_record_key_payload, identity_context, @@ -46,8 +46,10 @@ use crate::bindings::_helpers::records::fields::field as native_field; use crate::bindings::_helpers::records::fields::mapping_get_or; use crate::bindings::_helpers::records::fields::path_posix_text; use crate::bindings::_helpers::records::ids::{ErrorId, RecordId}; -use crate::bindings::_helpers::sql_test_glue::plan_driver::plan_compiled_sql_test_artifacts; +use crate::bindings::_helpers::sql_test_glue::models::SqlTestPlanOptions; +use crate::bindings::_helpers::sql_test_glue::plan_driver::planned_rows; use crate::bindings::_helpers::sql_test_glue::plan_driver::sql_test_model_chain_names_by_key; +use crate::bindings::_helpers::sql_test_glue::planning::PlanRow; const COMPILED_DIR: &str = "compiled"; const MODELS_DIR: &str = "models"; @@ -352,24 +354,24 @@ pub(crate) struct Prepared<'a, 'py> { type PendingTest = (Py, Option, Option); /// The managed SQL test artifact paths and the planning diagnostics of one write. -type WrittenTests<'py> = (Vec>, Vec>); +type WrittenTests<'py> = (Vec, Vec>); /// SQL tests whose cached artifacts cannot be reused, planned, beside the reusable ones. #[pyclass(module = "sqlbuild._native", frozen)] pub(crate) struct PlannedStaticSqlTests { - tests_root: Py, + tests_root: PathBuf, /// Each pending test with its cache slot and artifact identity, when caching. pending: Vec, - /// One planned `NativeSqlTestArtifact` per pending test. - artifacts: Vec>, - cached_paths: Vec>, + /// One native plan per pending test, with rendered SQL already validated. + artifacts: Vec, + cached_paths: Vec, cached_records: Vec<(String, SqlTestArtifactRecord)>, } /// `zip(pending, artifacts, strict=True)`. fn zipped( planned: &PlannedStaticSqlTests, -) -> PyResult)>> { +) -> PyResult> { if planned.pending.len() != planned.artifacts.len() { let relation: &str = if planned.artifacts.len() < planned.pending.len() { "shorter" @@ -388,7 +390,7 @@ fn planned_artifacts<'py>( adapter: &Bound<'py, PyAny>, project: &Bound<'py, PyAny>, tests: Vec>, -) -> PyResult>> { +) -> PyResult> { let py: Python<'py> = project.py(); if tests.is_empty() { return Ok(Vec::new()); @@ -402,24 +404,19 @@ fn planned_artifacts<'py>( )?; within(&lifecycle, |entered| { count_stage(py, SQL_TEST_ARTIFACT_PLANS, 1)?; - let artifacts: Vec> = plan_compiled_sql_test_artifacts( - py, - Some(keywords( - py, - &[ - ("project", project.clone()), - ("tests", PyTuple::new(py, tests)?.into_any()), - ("adapter", adapter.clone()), - ( - "sql_analysis_enabled", - native_field(&native_field(project, "settings")?, "sql_analysis")?, - ), - ], - )?), - )? - .try_iter()? - .map(|artifact| artifact.map(Bound::unbind)) - .collect::>()?; + let artifacts = planned_rows( + (project, adapter), + &PyTuple::new(py, tests)?.into_any(), + &SqlTestPlanOptions { + sql_analysis_enabled: native_field( + &native_field(project, "settings")?, + "sql_analysis", + )?, + render_sql: true, + include_plan: false, + artifact: true, + }, + )?; let metadata: Bound<'py, PyDict> = PyDict::new(py); metadata.set_item("item_count", artifacts.len())?; let _ = call_with( @@ -471,7 +468,7 @@ fn log_cache_misses(py: Python<'_>, misses: &BTreeMap<&'static str, usize>) -> P } /// Plan every SQL test whose cached artifact under `target_dir` cannot be reused. -fn plan_tests<'py>( +pub(crate) fn plan_tests<'py>( target_dir: &Bound<'py, PyAny>, (adapter, project): (&Bound<'py, PyAny>, &Bound<'py, PyAny>), recorder: ArtifactRecorder<'_>, @@ -503,7 +500,7 @@ fn plan_tests<'py>( .collect(), }; let mut pending: Vec = Vec::new(); - let mut cached_paths: Vec> = Vec::new(); + let mut cached_paths: Vec = Vec::new(); let mut cached_records: Vec<(String, SqlTestArtifactRecord)> = Vec::new(); let mut misses: BTreeMap<&'static str, usize> = BTreeMap::new(); for (index, (test, identity)) in tests.into_iter().zip(identities).enumerate() { @@ -522,9 +519,9 @@ fn plan_tests<'py>( *misses.entry(reason).or_default() += 1; } if let Ok(record) = reusable { - let path: Bound<'py, PyAny> = tests_root.div(&record.relative_path)?; - artifact_kept(recorder, &path, (record.size, record.mtime_ns))?; - cached_paths.push(path.unbind()); + let path = tests_root_path.join(&record.relative_path); + artifact_kept(py, recorder, &path, (record.size, record.mtime_ns))?; + cached_paths.push(path); cached_records.push((record_key, record)); } else { pending.push((test.unbind(), Some(record_key), Some(identity))); @@ -538,9 +535,9 @@ fn plan_tests<'py>( .iter() .map(|(test, _, _)| test.bind(py).clone()) .collect(); - let artifacts: Vec> = planned_artifacts(adapter, project, pending_tests)?; + let artifacts = planned_artifacts(adapter, project, pending_tests)?; Ok(PlannedStaticSqlTests { - tests_root: tests_root.unbind(), + tests_root: tests_root_path, pending, artifacts, cached_paths, @@ -562,7 +559,8 @@ fn resolved_plan<'py>( if planned .get() .tests_root - .bind(py) + .clone() + .into_bound_py_any(py)? .eq(joined(target_dir, &[COMPILED_DIR, TESTS_DIR])?)? { return Ok(planned); @@ -578,11 +576,11 @@ fn resolved_plan<'py>( /// The planning errors of one test's artifact, as SQL test diagnostics. fn artifact_diagnostics<'py>( test: &Bound<'py, PyAny>, - artifact: &Bound<'py, PyAny>, + artifact: &PlanRow, ) -> PyResult>> { let py: Python<'py> = test.py(); let mut diagnostics: Vec> = Vec::new(); - for message in native_field(artifact, "error_messages")?.try_iter()? { + for message in &artifact.error_messages { diagnostics.push(new_record_with( py, RecordId::CompilerDiagnostic, @@ -593,7 +591,7 @@ fn artifact_diagnostics<'py>( "code", native_field(&error_class(py, ErrorId::PlannerInput)?, "code")?, ), - ("message", message?), + ("message", message.into_bound_py_any(py)?), ("resource_type", "sql_test".into_bound_py_any(py)?), ("resource_name", native_field(test, "name")?), ("path", native_field(test, "source_path")?), @@ -614,39 +612,32 @@ fn write_static_tests<'py>( let planned: Bound<'py, PlannedStaticSqlTests> = resolved_plan(target_dir, (adapter, project), prepared)?; let planned: &PlannedStaticSqlTests = planned.get(); - let tests_root: &Bound<'py, PyAny> = planned.tests_root.bind(py); - let tests_root_path: PathBuf = tests_root.extract()?; - let mut managed: Vec> = planned - .cached_paths - .iter() - .map(|path| path.bind(py).clone()) - .collect(); + let tests_root = &planned.tests_root; + let mut managed = planned.cached_paths.clone(); let mut records: Vec<(String, SqlTestArtifactRecord)> = planned.cached_records.clone(); - let mut test_paths: Vec> = Vec::new(); + let mut test_paths: Vec = Vec::new(); for ((test, _, _), artifact) in zipped(planned)? { - let artifact: &Bound<'py, PyAny> = artifact.bind(py); - let path: Bound<'py, PyAny> = tests_root.div(compiled_sql_test_path( + let path = tests_root.join(compiled_sql_test_path( test.bind(py), - &native_field(artifact, "model_names")?, - )?)?; - batch.queue_sql(&path, &native_field(artifact, "sql")?.extract::()?)?; + artifact.plan.model_names.clone(), + )?); + batch.queue_sql_at(py, path.clone(), artifact.rendered_sql(py)?)?; managed.push(path.clone()); test_paths.push(path); } batch.flush(py)?; let mut diagnostics: Vec> = Vec::new(); for (((test, record_key, identity), artifact), path) in zipped(planned)?.zip(&test_paths) { - let found: Vec> = artifact_diagnostics(test.bind(py), artifact.bind(py))?; + let found: Vec> = artifact_diagnostics(test.bind(py), artifact)?; if !found.is_empty() { diagnostics.extend(found); continue; } - if let (Some(record_key), Some(identity)) = (record_key, identity) { - let artifact_path: PathBuf = path.extract()?; - if let Some(record) = new_sql_test_record(&tests_root_path, &artifact_path, identity) { - records.retain(|(key, _)| key != record_key); - records.push((record_key.clone(), record)); - } + if let (Some(record_key), Some(identity)) = (record_key, identity) + && let Some(record) = new_sql_test_record(tests_root, path, identity) + { + records.retain(|(key, _)| key != record_key); + records.push((record_key.clone(), record)); } } let cache_dir: Bound<'py, PyAny> = native_field(project, "compile_cache_dir")?; @@ -727,7 +718,7 @@ pub(crate) fn write_static_target<'py>( write_static_tests(target_dir, (adapter, project), prepared, &mut batch) })(); let (test_paths, diagnostics) = batch.settle(py, queued)?; - managed.extend(test_paths); + managed_models.extend(test_paths); if remove_stale_files { let mut paths: Vec = managed_models; paths.extend(extracted(&managed)?); @@ -927,7 +918,7 @@ pub(crate) fn planning_diagnostics<'py>( resolved_plan(target_dir, (adapter, project), prepared)?; let mut diagnostics: Vec> = Vec::new(); for ((test, _, _), artifact) in zipped(planned.get())? { - diagnostics.extend(artifact_diagnostics(test.bind(py), artifact.bind(py))?); + diagnostics.extend(artifact_diagnostics(test.bind(py), artifact)?); } PyTuple::new(py, diagnostics) } @@ -1039,55 +1030,7 @@ pub(crate) fn write_compile_target<'py>( ) } -/// Plan uncached SQL tests and report their errors without writing artifacts or cache. -#[pyfunction] -#[pyo3(signature = (**options))] -pub(crate) fn static_sql_test_planning_diagnostics<'py>( - py: Python<'py>, - options: Option>, -) -> PyResult> { - let arguments: KeywordArguments<'py> = KeywordArguments::read( - py, - "static_sql_test_planning_diagnostics", - options, - (&["target_dir", "adapter", "project"], &[]), - )?; - planning_diagnostics( - &arguments.value("target_dir")?, - (&arguments.value("adapter")?, &arguments.value("project")?), - Prepared::default(), - ) -} - -/// Plan every SQL test whose cached artifact under `target_dir` cannot be reused. -#[pyfunction] -#[pyo3(signature = (**options))] -pub(crate) fn plan_static_sql_tests<'py>( - py: Python<'py>, - options: Option>, -) -> PyResult { - let arguments: KeywordArguments<'py> = KeywordArguments::read( - py, - "plan_static_sql_tests", - options, - (&["target_dir", "adapter", "project"], &["artifact_writes"]), - )?; - let recorder: Option> = - arguments.value("artifact_writes")?.extract()?; - plan_tests( - &arguments.value("target_dir")?, - (&arguments.value("adapter")?, &arguments.value("project")?), - recorder.as_ref(), - ) -} - pub(crate) fn register(module: &Bound<'_, PyModule>) -> PyResult<()> { - module.add_class::()?; module.add_function(wrap_pyfunction!(write_static_compile_target, module)?)?; - module.add_function(wrap_pyfunction!(write_compile_target, module)?)?; - module.add_function(wrap_pyfunction!( - static_sql_test_planning_diagnostics, - module - )?)?; - module.add_function(wrap_pyfunction!(plan_static_sql_tests, module)?) + module.add_function(wrap_pyfunction!(write_compile_target, module)?) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/runtime_target.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/runtime_target.rs index 304afb25e6..b039b783bd 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/runtime_target.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/runtime_target.rs @@ -1,6 +1,7 @@ //! SQL test artifact paths, and the executed SQL and Python check results written to `target/run`. use std::collections::{HashMap, HashSet}; +use std::path::PathBuf; use pyo3::prelude::{ Bound, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyResult, Python, pyfunction, @@ -87,14 +88,14 @@ pub(crate) fn planned_sql_test_path<'py>(entry: &Bound<'py, PyAny>) -> PyResult< } /// The target path of one compiled SQL test from its chain's model names. -pub(crate) fn compiled_sql_test_path<'py>( - test: &Bound<'py, PyAny>, - model_names: &Bound<'py, PyAny>, -) -> PyResult> { - path_object( - test.py(), - &sql_test_output_path(&path_facts(test, model_names.extract()?)?), - ) +pub(crate) fn compiled_sql_test_path( + test: &Bound<'_, PyAny>, + model_names: Vec, +) -> PyResult { + Ok(PathBuf::from(sql_test_output_path(&path_facts( + test, + model_names, + )?))) } fn write_text(path: &Bound<'_, PyAny>, text: &str) -> PyResult<()> { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/records/ids.rs b/crates/sqlbuild-python/src/bindings/_helpers/records/ids.rs index 86654230fb..15756e1c8e 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/records/ids.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/records/ids.rs @@ -861,7 +861,6 @@ native_ids! { PlanRuleRun => "plan_rule_run", PlanSqlTests => "plan_sql_tests", PlanSqlTestsNatively => "plan_sql_tests_natively", - PlanStaticSqlTests => "plan_static_sql_tests", PlanningProgressReporter => "PlanningProgressReporter", PreparedCompileArtifacts => "PreparedCompileArtifacts", ProjectWith => "project_with", @@ -969,7 +968,6 @@ native_ids! { SqlTestModelChainNamesByKey => "sql_test_model_chain_names_by_key", SqlTestScanCache => "SqlTestScanCache", SqlValueValidationError => "SqlValueValidationError", - StaticSqlTestPlanningDiagnostics => "static_sql_test_planning_diagnostics", StoredOldNameViewColumns => "stored_old_name_view_columns", TerminalEventIndex => "TerminalEventIndex", TerminalEventIndexContext => "_terminal_event_index_context", diff --git a/crates/sqlbuild-python/src/bindings/_helpers/refactor/offline_compile.rs b/crates/sqlbuild-python/src/bindings/_helpers/refactor/offline_compile.rs index 0ff7c2451d..13664305bf 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/refactor/offline_compile.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/refactor/offline_compile.rs @@ -8,7 +8,7 @@ use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyErr, PyResult, Python}; use pyo3::types::{PyBool, PyString, PyTuple}; use crate::bindings::_helpers::boundary::python_calls::keywords; -use crate::bindings::_helpers::compile_command::target_writer::static_sql_test_planning_diagnostics; +use crate::bindings::_helpers::compile_command::target_writer::{Prepared, planning_diagnostics}; use crate::bindings::_helpers::compile_driver::analysis::analyze_compile_project; use crate::bindings::_helpers::records::construct::error_class; use crate::bindings::_helpers::records::construct::{enum_member, new_record, new_record_with}; @@ -171,22 +171,13 @@ pub(crate) fn compile_for_refactor<'py>( only_rule_failures &= native_field(error, "phase")?.is(&rule_phase); } if only_rule_failures { - let planned: PyResult> = static_sql_test_planning_diagnostics( - py, - Some(keywords( - py, - &[ - ( - "target_dir", - project_dir.join("target").into_pyobject(py)?.into_any(), - ), - ("adapter", native_field(&analysis, "adapter")?), - ( - "project", - native_field(&native_field(&analysis, "graph")?, "project")?, - ), - ], - )?), + let planned: PyResult> = planning_diagnostics( + &project_dir.join("target").into_pyobject(py)?.into_any(), + ( + &native_field(&analysis, "adapter")?, + &native_field(&native_field(&analysis, "graph")?, "project")?, + ), + Prepared::default(), ); match planned { Ok(diagnostics) => errors.extend(error_diagnostics(diagnostics.into_any())?), diff --git a/crates/sqlbuild-python/src/bindings/_helpers/sql_test_glue/plan_driver.rs b/crates/sqlbuild-python/src/bindings/_helpers/sql_test_glue/plan_driver.rs index e58be88dda..aa22fbe797 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/sql_test_glue/plan_driver.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/sql_test_glue/plan_driver.rs @@ -13,9 +13,23 @@ use crate::bindings::_helpers::records::ids::{ErrorId, RecordId}; use crate::bindings::_helpers::sql_test_glue::models::SqlTestPlanOptions; use crate::bindings::_helpers::sql_test_glue::plan_values::tuple; use crate::bindings::_helpers::sql_test_glue::planning::{ - ChainRequest, PlanningRequest, plan_compiled_sql_tests, resolve_compiled_sql_test_chains, + ChainRequest, PlanRow, PlanningRequest, plan_compiled_sql_tests, + resolve_compiled_sql_test_chains, }; +impl PlanRow { + /// Rendered SQL for an artifact, or the planning error shared by native and Python consumers. + pub(crate) fn rendered_sql(&self, py: Python<'_>) -> PyResult<&str> { + let Some(sql) = self.plan.sql.as_deref() else { + return Err(PyErr::from_value( + error_class(py, ErrorId::NativeSqlTestPlanning)? + .call1(("native SQL-test planning omitted rendered SQL",))?, + )); + }; + Ok(sql) + } +} + pub(super) fn chains<'py>( project: &Bound<'py, PyAny>, tests: &Bound<'py, PyAny>, @@ -58,16 +72,16 @@ pub(crate) fn resolve_sql_test_model_chains<'py>( chains(project, tests) } -/// Plans of one SQL-test batch, or their compiled artifacts. -pub(crate) fn planned<'py>( +/// Native plans of one SQL-test batch, with adapter rendering and timing accounted for. +pub(crate) fn planned_rows<'py>( (project, adapter): (&Bound<'py, PyAny>, &Bound<'py, PyAny>), tests: &Bound<'py, PyAny>, options: &SqlTestPlanOptions<'py>, -) -> PyResult> { +) -> PyResult> { let py = project.py(); let artifact = options.artifact; if !tests.is_truthy()? { - return Ok(PyTuple::empty(py)); + return Ok(Vec::new()); } let start: u64 = imported(py, "time", "perf_counter_ns")? .call0()? @@ -125,17 +139,27 @@ pub(crate) fn planned<'py>( ] { add_compile_timing(phase, elapsed); } + if artifact { + for row in &rows { + row.rendered_sql(py)?; + } + } + Ok(rows) +} + +/// Plans of one SQL-test batch, or their compiled artifacts, for Python consumers. +pub(crate) fn planned<'py>( + (project, adapter): (&Bound<'py, PyAny>, &Bound<'py, PyAny>), + tests: &Bound<'py, PyAny>, + options: &SqlTestPlanOptions<'py>, +) -> PyResult> { + let py = project.py(); + let rows = planned_rows((project, adapter), tests, options)?; let mut plans: Vec> = Vec::new(); for row in rows { let plan = super::plan_rows::plan(py, row)?; - if artifact { + if options.artifact { let sql = native_field(&plan, "sql")?; - if sql.is_none() { - return Err(PyErr::from_value( - error_class(py, ErrorId::NativeSqlTestPlanning)? - .call1(("native SQL-test planning omitted rendered SQL",))?, - )); - } plans.push(new_record_with( py, RecordId::NativeSqlTestArtifact, diff --git a/fensu.toml b/fensu.toml index b933e33f59..2710903f08 100644 --- a/fensu.toml +++ b/fensu.toml @@ -2302,7 +2302,7 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/compile_command/prepared_artifacts.rs=7", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/report.rs=6", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/reuse_recording.rs=7", - "crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs=4", + "crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/compile_context/diagnostic_collector.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/compile_context/input_reads.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/compile_driver/analysis.rs=12", From fe12688517492516c34c0f54225eeed46e8cf15c Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 30 Oct 2026 00:27:55 +0000 Subject: [PATCH 4/8] refactor: assemble SQL-test cache identities natively Keep cache payload objects, CTE arrays and sorted pairs as Rust JSON values. Convert record and adapter leaf values at the existing JSON boundary, preserving the version-5 digest format and scalar checks. Amp-Thread-ID: https://ampcode.com/threads/T-01a18715-4e00-708e-a87d-6bd109748d66 --- .../compile_command/test_artifact_cache.rs | 416 +++++++++--------- .../_helpers/compile_outputs/json_report.rs | 11 +- fensu.toml | 2 +- 3 files changed, 205 insertions(+), 224 deletions(-) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs index 77e3419d92..abf1e89464 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs @@ -3,18 +3,17 @@ use std::collections::HashMap; use std::path::PathBuf; -use pyo3::IntoPyObjectExt; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python}; -use pyo3::types::{PyDict, PyDictMethods, PyList, PyListMethods, PyTuple}; use rayon::prelude::{IntoParallelRefIterator, ParallelIterator}; use sqlbuild_cache::artifacts::main::read_sql_test_cache::read_sql_test_cache; use sqlbuild_cache::artifacts::main::write_sql_test_cache::write_sql_test_cache; use sqlbuild_cache::artifacts::models::SqlTestArtifactRecord; use sqlbuild_cache::digest::main::bytes_digest::bytes_digest; use sqlbuild_cache::digest::main::hex_digest::hex_digest; +use sqlbuild_core::json::models::JsonValue; use crate::bindings::_helpers::boundary::python_calls::imported; -use crate::bindings::_helpers::compile_outputs::json_report::orjson_sorted; +use crate::bindings::_helpers::compile_outputs::json_report::{orjson_sorted, orjson_value}; use crate::bindings::_helpers::records::construct::record_class; use crate::bindings::_helpers::records::ids::RecordId; @@ -29,11 +28,6 @@ pub(crate) struct IdentityContext { model_identities: HashMap, } -/// `orjson.dumps(payload, option=OPT_SORT_KEYS)`, the text an identity digest is taken of. -fn payload_text(payload: &Bound<'_, PyAny>) -> PyResult { - orjson_sorted(payload) -} - /// The hexadecimal BLAKE3 digest of each payload text, hashed without holding the interpreter. pub(crate) fn payload_digests(py: Python<'_>, payloads: &[String]) -> Vec { py.detach(|| { @@ -51,71 +45,73 @@ pub(crate) fn yield_interpreter(py: Python<'_>, index: usize) { } } -fn dict<'py>( - py: Python<'py>, - entries: Vec<(&str, Bound<'py, PyAny>)>, -) -> PyResult> { - let built: Bound<'py, PyDict> = PyDict::new(py); - for (key, value) in entries { - built.set_item(key, value)?; - } - Ok(built) +/// One identity object, assembled without allocating temporary Python containers. +fn object(entries: Vec<(&str, JsonValue)>) -> JsonValue { + JsonValue::Object( + entries + .into_iter() + .map(|(key, value)| (key.to_owned(), value)) + .collect(), + ) +} + +/// A compiled record's field converted at its boundary, retaining orjson's scalar checks. +fn json_field(value: &Bound<'_, PyAny>, name: &str) -> PyResult { + orjson_value(&value.getattr(name)?) } /// String pairs ordered lexicographically, as Python orders these cache inputs. -fn sorted_pairs(py: Python<'_>, mut items: Vec<(String, String)>) -> PyResult> { +fn sorted_pairs(mut items: Vec<(String, String)>) -> JsonValue { items.sort(); - Ok(PyList::new(py, items)?.into_any()) + JsonValue::Array( + items + .into_iter() + .map(|(first, second)| { + JsonValue::Array(vec![JsonValue::String(first), JsonValue::String(second)]) + }) + .collect(), + ) } -fn package_version<'py>(py: Python<'py>, package: &str) -> PyResult> { +fn package_version(py: Python<'_>, package: &str) -> PyResult { match imported(py, METADATA_MODULE, "version")?.call1((package,)) { - Ok(version) => Ok(version), + Ok(version) => orjson_value(&version), Err(error) if error.is_instance(py, &imported(py, METADATA_MODULE, "PackageNotFoundError")?) => { - "unknown".into_bound_py_any(py) + Ok(JsonValue::String("unknown".to_owned())) } Err(error) => Err(error), } } /// `sorted((str(dep.resource_type), dep.name) for dep in deps)`. -fn sorted_deps<'py>(deps: &Bound<'py, PyAny>) -> PyResult> { - let py: Python<'py> = deps.py(); +fn sorted_deps(deps: &Bound<'_, PyAny>) -> PyResult { let mut pairs: Vec<(String, String)> = Vec::new(); for dep in deps.try_iter()? { - let dep: Bound<'py, PyAny> = dep?; + let dep = dep?; pairs.push(( dep.getattr("resource_type")?.str()?.extract()?, dep.getattr("name")?.extract()?, )); } - sorted_pairs(py, pairs) + Ok(sorted_pairs(pairs)) } fn model_identity_payload(model: &Bound<'_, PyAny>) -> PyResult { - let py: Python<'_> = model.py(); let values: Bound<'_, PyAny> = model.getattr("config")?.getattr("values")?; - payload_text( - dict( - py, - vec![ - ("name", model.getattr("name")?), - ("query_sql", model.getattr("query_sql")?), - ("deps", sorted_deps(&model.getattr("deps")?)?), - ( - "cursor", - ( - values.call_method1("get", ("cursor_type",))?, - values.call_method1("get", ("cursor_grain",))?, - ) - .into_bound_py_any(py)?, - ), - ], - )? - .as_any(), - ) + orjson_sorted(&object(vec![ + ("name", json_field(model, "name")?), + ("query_sql", json_field(model, "query_sql")?), + ("deps", sorted_deps(&model.getattr("deps")?)?), + ( + "cursor", + JsonValue::Array(vec![ + orjson_value(&values.call_method1("get", ("cursor_type",))?)?, + orjson_value(&values.call_method1("get", ("cursor_grain",))?)?, + ]), + ), + ])) } /// Hash adapter and model inputs once for every test artifact identity. @@ -140,56 +136,55 @@ pub(crate) fn identity_context<'py>( .import("platform")? .call_method0("python_version_tuple")? .get_item(pyo3::types::PySlice::new(py, 0, 2, 1))?; - let common: Bound<'py, PyDict> = dict( - py, - vec![ - ("algorithm", ALGORITHM_FINGERPRINT.into_bound_py_any(py)?), - ("sqlbuild_version", package_version(py, "sqlbuild")?), - ( - "polyglot_version", - package_version(py, "polyglot-sql-chio")?, - ), - ("python_version", version_tuple), - ( - "adapter_class", - format!( - "{}.{}", - adapter_type.getattr("__module__")?.str()?, - adapter_type.getattr("__qualname__")?.str()? - ) - .into_bound_py_any(py)?, - ), - ( - "set_difference_operator", - adapter.call_method0("render_set_difference_operator")?, - ), - ( - "sql_analysis_dialect", - adapter.call_method0("sql_analysis_dialect")?, - ), - ( - "requires_derived_table_aliases", - adapter.call_method0("requires_derived_table_aliases")?, - ), - ( - "sql_analysis_enabled", - project.getattr("settings")?.getattr("sql_analysis")?, - ), - ( - "effective_target_name", - project.getattr("effective_target_name")?, - ), - ( - "effective_target_database", - project.getattr("effective_target_database")?, - ), - ( - "effective_target_schema", - project.getattr("effective_target_schema")?, - ), - ("function_locations", sorted_pairs(py, functions)?), - ], - )?; + let common = object(vec![ + ( + "algorithm", + JsonValue::String(ALGORITHM_FINGERPRINT.to_owned()), + ), + ("sqlbuild_version", package_version(py, "sqlbuild")?), + ( + "polyglot_version", + package_version(py, "polyglot-sql-chio")?, + ), + ("python_version", orjson_value(&version_tuple)?), + ( + "adapter_class", + JsonValue::String(format!( + "{}.{}", + adapter_type.getattr("__module__")?.str()?, + adapter_type.getattr("__qualname__")?.str()? + )), + ), + ( + "set_difference_operator", + orjson_value(&adapter.call_method0("render_set_difference_operator")?)?, + ), + ( + "sql_analysis_dialect", + orjson_value(&adapter.call_method0("sql_analysis_dialect")?)?, + ), + ( + "requires_derived_table_aliases", + orjson_value(&adapter.call_method0("requires_derived_table_aliases")?)?, + ), + ( + "sql_analysis_enabled", + json_field(&project.getattr("settings")?, "sql_analysis")?, + ), + ( + "effective_target_name", + json_field(project, "effective_target_name")?, + ), + ( + "effective_target_database", + json_field(project, "effective_target_database")?, + ), + ( + "effective_target_schema", + json_field(project, "effective_target_schema")?, + ), + ("function_locations", sorted_pairs(functions)), + ]); let mut names: Vec = Vec::new(); let mut payloads: Vec = Vec::new(); for (index, model) in project.getattr("models")?.try_iter()?.enumerate() { @@ -198,7 +193,7 @@ pub(crate) fn identity_context<'py>( payloads.push(model_identity_payload(&model)?); yield_interpreter(py, index); } - payloads.push(payload_text(common.as_any())?); + payloads.push(orjson_sorted(&common)?); let mut digests: Vec = payload_digests(py, &payloads); let common_identity: String = digests.pop().unwrap_or_default(); let model_identities: HashMap = names.into_iter().zip(digests).collect(); @@ -209,110 +204,102 @@ pub(crate) fn identity_context<'py>( } /// `[(cte.name, cte.sql_body) for cte in ctes]`. -fn cte_payload<'py>(ctes: &Bound<'py, PyAny>) -> PyResult> { - let py: Python<'py> = ctes.py(); - let rows: Bound<'py, PyList> = PyList::empty(py); +fn cte_payload(ctes: &Bound<'_, PyAny>) -> PyResult { + let mut rows: Vec = Vec::new(); for cte in ctes.try_iter()? { - let cte: Bound<'py, PyAny> = cte?; - rows.append((cte.getattr("name")?, cte.getattr("sql_body")?))?; + rows.push(cte_pair(&cte?)?); } - Ok(rows.into_any()) + Ok(JsonValue::Array(rows)) +} + +fn cte_pair(cte: &Bound<'_, PyAny>) -> PyResult { + Ok(JsonValue::Array(vec![ + json_field(cte, "name")?, + json_field(cte, "sql_body")?, + ])) } /// `sorted(mapping.items())`. -fn sorted_items<'py>(mapping: &Bound<'py, PyAny>) -> PyResult> { - let py: Python<'py> = mapping.py(); - sorted_pairs( - py, +fn sorted_items(mapping: &Bound<'_, PyAny>) -> PyResult { + Ok(sorted_pairs( mapping .call_method0("items")? .try_iter()? .map(|item| item?.extract::<(String, String)>()) .collect::>>()?, - ) + )) } -fn test_payload<'py>(test: &Bound<'py, PyAny>) -> PyResult> { - let py: Python<'py> = test.py(); - let payload: Bound<'py, PyDict> = dict( - py, - vec![ - ("case_fingerprint", test.getattr("case_fingerprint")?), - ("mode", test.getattr("mode")?.getattr("value")?), - ("sql_body", test.getattr("sql_body")?), - ("scope_deps", sorted_deps(&test.getattr("scope_deps")?)?), - ], - )?; - let test_payload: Bound<'py, PyAny> = test.getattr("payload")?; +fn test_payload(test: &Bound<'_, PyAny>) -> PyResult { + let py = test.py(); + let mut payload = vec![ + ("case_fingerprint", json_field(test, "case_fingerprint")?), + ("mode", json_field(&test.getattr("mode")?, "value")?), + ("sql_body", json_field(test, "sql_body")?), + ("scope_deps", sorted_deps(&test.getattr("scope_deps")?)?), + ]; + let test_payload = test.getattr("payload")?; if test_payload.is_instance(&record_class(py, RecordId::CompiledModelSqlTestPayload)?)? { - let block: Bound<'py, PyAny> = test.getattr("test_block")?; - payload.set_item( + let block = test.getattr("test_block")?; + payload.push(( "model", - dict( - py, - vec![ - ( - "authored_ctes", - cte_payload(&test_payload.getattr("authored_ctes")?)?, - ), - ( - "macro_mocks", - sorted_items(&test_payload.getattr("macro_mocks")?)?, - ), - ( - "model_query_overrides", - sorted_items(&test_payload.getattr("model_query_overrides")?)?, - ), - ( - "expected_model_names", - test_payload.getattr("expected_model_names")?, - ), - ( - "assertion_ctes", - cte_payload(&test_payload.getattr("assertion_ctes")?)?, - ), - ( - "cursor_window", - (block.getattr("cursor_start")?, block.getattr("cursor_end")?) - .into_bound_py_any(py)?, - ), - ], - )?, - )?; + object(vec![ + ( + "authored_ctes", + cte_payload(&test_payload.getattr("authored_ctes")?)?, + ), + ( + "macro_mocks", + sorted_items(&test_payload.getattr("macro_mocks")?)?, + ), + ( + "model_query_overrides", + sorted_items(&test_payload.getattr("model_query_overrides")?)?, + ), + ( + "expected_model_names", + json_field(&test_payload, "expected_model_names")?, + ), + ( + "assertion_ctes", + cte_payload(&test_payload.getattr("assertion_ctes")?)?, + ), + ( + "cursor_window", + JsonValue::Array(vec![ + json_field(&block, "cursor_start")?, + json_field(&block, "cursor_end")?, + ]), + ), + ]), + )); } else if test_payload.is_instance(&record_class( py, RecordId::CompiledDirectLogicSqlTestPayload, )?)? { - payload.set_item( + payload.push(( "direct", - dict( - py, - vec![ - ( - "helper_ctes", - cte_payload(&test_payload.getattr("helper_ctes")?)?, - ), - ( - "actual_cte", - cte_payload( - PyTuple::new(py, [test_payload.getattr("actual_cte")?])?.as_any(), - )?, - ), - ( - "expected_cte", - cte_payload( - PyTuple::new(py, [test_payload.getattr("expected_cte")?])?.as_any(), - )?, - ), - ( - "tested_resource_names", - test_payload.getattr("tested_resource_names")?, - ), - ], - )?, - )?; + object(vec![ + ( + "helper_ctes", + cte_payload(&test_payload.getattr("helper_ctes")?)?, + ), + ( + "actual_cte", + JsonValue::Array(vec![cte_pair(&test_payload.getattr("actual_cte")?)?]), + ), + ( + "expected_cte", + JsonValue::Array(vec![cte_pair(&test_payload.getattr("expected_cte")?)?]), + ), + ( + "tested_resource_names", + json_field(&test_payload, "tested_resource_names")?, + ), + ]), + )); } - Ok(payload) + Ok(object(payload)) } /// The text whose digest is the complete identity of one rendered comparison artifact. @@ -321,54 +308,43 @@ pub(crate) fn artifact_identity_payload( model_chain_names: &Bound<'_, PyAny>, context: &IdentityContext, ) -> PyResult { - let py: Python<'_> = test.py(); - let chain: Bound<'_, PyList> = PyList::empty(py); + let mut chain: Vec = Vec::new(); for name in model_chain_names.try_iter()? { let name: String = name?.extract()?; let identity: Option<&String> = context.model_identities.get(&name); - chain.append((name, identity))?; + chain.push(JsonValue::Array(vec![ + JsonValue::String(name), + identity.cloned().map_or(JsonValue::Null, JsonValue::String), + ])); } - payload_text( - dict( - py, - vec![ - ( - "common_identity", - context.common_identity.as_str().into_bound_py_any(py)?, - ), - ("test", test_payload(test)?.into_any()), - ("model_chain", chain.into_any()), - ], - )? - .as_any(), - ) + orjson_sorted(&object(vec![ + ( + "common_identity", + JsonValue::String(context.common_identity.clone()), + ), + ("test", test_payload(test)?), + ("model_chain", JsonValue::Array(chain)), + ])) } /// The text whose digest is the stable cache slot of one source test case. pub(crate) fn artifact_record_key_payload(test: &Bound<'_, PyAny>) -> PyResult { - let py: Python<'_> = test.py(); let source_path: Bound<'_, PyAny> = test.getattr("source_path")?; - payload_text( - dict( - py, - vec![ - ( - "source_path", - if source_path.is_none() { - source_path - } else { - source_path.call_method0("as_posix")? - }, - ), - ("block_index", test.getattr("block_index")?), - ("name", test.getattr("name")?), - ("parent_name", test.getattr("parent_name")?), - ("case_name", test.getattr("case_name")?), - ("case_index", test.getattr("case_index")?), - ], - )? - .as_any(), - ) + orjson_sorted(&object(vec![ + ( + "source_path", + if source_path.is_none() { + JsonValue::Null + } else { + orjson_value(&source_path.call_method0("as_posix")?)? + }, + ), + ("block_index", json_field(test, "block_index")?), + ("name", json_field(test, "name")?), + ("parent_name", json_field(test, "parent_name")?), + ("case_name", json_field(test, "case_name")?), + ("case_index", json_field(test, "case_index")?), + ])) } /// The valid cache records; a cache that cannot be read never fails compilation. diff --git a/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/json_report.rs b/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/json_report.rs index d729c5cc00..5c027d282d 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/json_report.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/compile_outputs/json_report.rs @@ -78,15 +78,20 @@ pub(crate) fn emit_json_report(report: &Bound<'_, PyAny>) -> PyResult { }) } -/// `orjson.dumps(value, option=OPT_SORT_KEYS)`, raising orjson's `TypeError`. -pub(crate) fn orjson_sorted(value: &Bound<'_, PyAny>) -> PyResult { +/// A Python value in orjson's supported domain, converted once at its data boundary. +pub(crate) fn orjson_value(value: &Bound<'_, PyAny>) -> PyResult { let mut conversion: Conversion = Conversion::default(); let converted: JsonValue = conversion.json_value(value)?; if let Some(error) = conversion.orjson_error { return Err(PyTypeError::new_err(error)); } + Ok(converted) +} + +/// `orjson.dumps(value, option=OPT_SORT_KEYS)` for an already-native payload. +pub(crate) fn orjson_sorted(value: &JsonValue) -> PyResult { emitted( - &converted, + value, &JsonDialect::Orjson(OrjsonOptions { indent_2: false, sort_keys: true, diff --git a/fensu.toml b/fensu.toml index 2710903f08..6ccc362a96 100644 --- a/fensu.toml +++ b/fensu.toml @@ -1741,7 +1741,7 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/compile_command/startup_inputs.rs=4", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/surrogates.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/compile_command/target_writer.rs=23", - "crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs=58", + "crates/sqlbuild-python/src/bindings/_helpers/compile_command/test_artifact_cache.rs=39", "crates/sqlbuild-python/src/bindings/_helpers/compile_context/compile_timings.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/compile_context/diagnostic_collector.rs=10", "crates/sqlbuild-python/src/bindings/_helpers/compile_context/input_reads.rs=3", From dbad87980b4e0bd715dfcb5097f05e3144fb435e Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 30 Oct 2026 00:37:59 +0000 Subject: [PATCH 5/8] refactor: retain native audit results through command completion Convert executor audit results once, share projection metadata with build reporting, and remove the completed JSON parse/rewrite step. Preserve public formatting and warehouse history behavior. Amp-Thread-ID: https://ampcode.com/threads/T-01a18715-4e00-708e-a87d-6bd109748d66 --- .../_helpers/audit_command/command.rs | 112 +++++++----------- .../_helpers/result_output/documents.rs | 42 ++++--- fensu.toml | 2 +- .../main/test_audit_command_storage.py | 22 +++- 4 files changed, 93 insertions(+), 85 deletions(-) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/audit_command/command.rs b/crates/sqlbuild-python/src/bindings/_helpers/audit_command/command.rs index bfa9c04ff5..aaa617d9ad 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/audit_command/command.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/audit_command/command.rs @@ -3,7 +3,7 @@ use std::collections::BTreeSet; use pyo3::prelude::{Bound, Py, PyAny, PyAnyMethods, PyModule, PyModuleMethods, PyResult, Python}; -use pyo3::types::{PyDict, PyDictMethods, PyInt, PyList, PyListMethods, PyString, PyTuple}; +use pyo3::types::{PyDict, PyDictMethods, PyInt, PyString}; use pyo3::{pyclass, pyfunction, pymethods, wrap_pyfunction}; use sqlbuild_analysis::assembly::target_checks::constants::PRESERVE_TARGET_VALUE; use sqlbuild_core::audit_command::main::audit_exit_code::audit_exit_code; @@ -12,6 +12,7 @@ use sqlbuild_core::audit_command::main::render_audit_summary::render_audit_summa use sqlbuild_core::audit_command::models::AuditCounts; use sqlbuild_core::cli_text::models::CliStyle; use sqlbuild_host::entries::models::ExecutorHost; +use sqlbuild_host::models::{AuditExecutionResult, AuditOutcome}; use crate::bindings::_helpers::boundary::python_calls::{call_with, keywords, within}; use crate::bindings::_helpers::build_progress::audit_progress::AuditProgressReporter; @@ -25,20 +26,17 @@ use crate::bindings::_helpers::compile_driver::static_context::compile_static_co use crate::bindings::_helpers::cost_telemetry::context::CostContext; use crate::bindings::_helpers::dbt_interop::reference_resolver::resolve_external_sql_reference_resolver; use crate::bindings::_helpers::native_host::host; +use crate::bindings::_helpers::native_host::results::ReadResult; use crate::bindings::_helpers::planner_entries::command_plans::build_audit_command_plan; use crate::bindings::_helpers::records::construct::{new_record_with, record_class}; use crate::bindings::_helpers::records::fields::field as native_field; use crate::bindings::_helpers::records::ids::RecordId; -use crate::bindings::_helpers::result_output::documents::format_audit_execution_json; -use crate::bindings::_helpers::result_output::documents::write_execution_json_output; +use crate::bindings::_helpers::result_output::documents::{ + audit_execution_json, audit_projection, write_execution_json_output, +}; use crate::bindings::_helpers::result_output::event_writer::ExecutionEventWriter; const EXECUTION_COMMAND: &str = "sqb audit"; -const PASS_OUTCOME: &str = "pass"; -const WARN_OUTCOME: &str = "warn"; -const ERROR_OUTCOME: &str = "error"; -const INSUFFICIENT_OUTCOME: &str = "insufficient"; -const PROJECTION_FAILURE: &str = "audit_result_projection_failure"; fn text<'py>(py: Python<'py>, value: &str) -> Bound<'py, PyAny> { PyString::new(py, value).into_any() @@ -292,7 +290,7 @@ fn execute<'py>( invocation: &Bound<'py, PyAny>, pipeline: &Bound<'py, PyAny>, run: &AuditRun<'py>, -) -> PyResult>> { +) -> PyResult>>> { let py: Python<'py> = invocation.py(); let plan: Bound<'py, PyAny> = native_field(pipeline, "plan_output")?; let project: Bound<'py, PyAny> = native_field(pipeline, "project")?; @@ -307,7 +305,7 @@ fn execute<'py>( connection: run.connection.clone().unbind(), }, )?; - let executed: PyResult>> = (|| { + let executed: PyResult>>> = (|| { run.progress .call_method1("set_result_callback", (native_field(&relay, "on_result")?,))?; let projector: Bound<'py, PyAny> = current_native_progress_projector(py)?; @@ -407,12 +405,13 @@ fn execute<'py>( ], )? .try_iter()? + .map(|result| AuditExecutionResult::read(&result?)) .collect() }) })(); let progress_closed: PyResult> = run.progress.call_method0("close"); let writer_closed: PyResult> = event_writer.call_method0("close"); - let results: Vec> = executed?; + let results = executed?; progress_closed?; writer_closed?; Ok(results) @@ -420,46 +419,33 @@ fn execute<'py>( /// The audit JSON document, with the result projection counts when audits were projected. fn audit_payload<'py>( - results: &Bound<'py, PyAny>, + py: Python<'py>, + results: &[AuditExecutionResult>], (concurrency, worker_count): (i64, i64), -) -> PyResult> { - let py: Python<'py> = results.py(); - let payload: Bound<'py, PyAny> = format_audit_execution_json( - py, - results.clone(), - Some(PyInt::new(py, concurrency).into_any()).filter(|value| !value.is_none()), - Some(PyInt::new(py, worker_count).into_any()).filter(|value| !value.is_none()), - ) - .and_then(|value| pyo3::IntoPyObjectExt::into_bound_py_any(value, py))?; +) -> PyResult { let projection: Bound<'py, PyAny> = host::entry(py, ExecutorHost::CurrentAuditResultProjection)?.call0()?; - if projection.is_none() { - return Ok(payload); - } - let json: Bound<'py, PyAny> = py.import("json")?.into_any(); - let document: Bound<'py, PyAny> = json.call_method1("loads", (payload,))?; - let counts: Bound<'py, PyDict> = PyDict::new(py); - for field in ["attempted_count", "written_count", "failed_count"] { - counts.set_item(field, native_field(&projection, field)?)?; - } - document.set_item("audit_result_projection", &counts)?; - if native_field(&projection, "degraded")?.is_truthy()? { - document.set_item("projection_degraded", true)?; - let reason: Bound<'py, PyDict> = PyDict::new(py); - reason.set_item("reason", PROJECTION_FAILURE)?; - for field in ["attempted_count", "written_count", "failed_count"] { - reason.set_item(field, native_field(&projection, field)?)?; - } - let reasons: Bound<'py, PyList> = PyList::empty(py); - reasons.append(reason)?; - document.set_item("projection_degradation_reasons", reasons)?; - } - let options: Bound<'py, PyDict> = PyDict::new(py); - options.set_item("indent", 2)?; - let dumped: String = json - .call_method("dumps", (document,), Some(&options))? - .extract()?; - Ok(PyString::new(py, &format!("{dumped}\n")).into_any()) + let trailing = if projection.is_none() { + Vec::new() + } else { + audit_projection( + py, + ( + native_field(&projection, "attempted_count")?.extract()?, + native_field(&projection, "written_count")?.extract()?, + native_field(&projection, "failed_count")?.extract()?, + ), + )? + }; + audit_execution_json( + py, + results, + ( + Some(PyInt::new(py, concurrency).into_any()), + Some(PyInt::new(py, worker_count).into_any()), + ), + trailing, + ) } /// The warning line for audit results that could not be stored in the warehouse history table; audit outcomes are unaffected. @@ -482,7 +468,7 @@ fn projection_warning(py: Python<'_>, style: CliStyle) -> PyResult, invocation: &Bound<'_, PyAny>, - results: &[Bound<'_, PyAny>], + results: &[AuditExecutionResult>], run: &AuditRun<'_>, ) -> PyResult { let py: Python<'_> = request.py(); @@ -491,16 +477,12 @@ fn complete( ..AuditCounts::default() }; for result in results { - match native_field(result, "outcome")? - .getattr("value")? - .extract::()? - .as_str() - { - PASS_OUTCOME => counts.passed += 1, - WARN_OUTCOME => counts.warned += 1, - ERROR_OUTCOME => counts.failed += 1, - INSUFFICIENT_OUTCOME => counts.insufficient += 1, - _ => {} + match result.outcome { + AuditOutcome::Pass => counts.passed += 1, + AuditOutcome::Warn => counts.warned += 1, + AuditOutcome::Error => counts.failed += 1, + AuditOutcome::Insufficient => counts.insufficient += 1, + AuditOutcome::Skipped => {} } } let stream: Bound<'_, PyAny> = native_field(invocation, "progress_stream")?; @@ -513,17 +495,13 @@ fn complete( (format!("{warning}{}", render_audit_summary(counts, style)),), )?; stream.call_method0("flush")?; - let payload: Bound<'_, PyAny> = audit_payload( - &PyTuple::new(py, results)?.into_any(), - (run.concurrency, run.worker_count), - )?; + let payload = audit_payload(py, results, (run.concurrency, run.worker_count))?; write_execution_json_output( py, - payload, + PyString::new(py, &payload).into_any(), native_field(request, "json_output")?.extract::()?, native_field(request, "json_output_path")?, - ) - .and_then(|value| pyo3::IntoPyObjectExt::into_bound_py_any(value, py))?; + )?; Ok(audit_exit_code(counts.failed)) } @@ -539,7 +517,7 @@ pub(crate) fn run_audit_command(py: Python<'_>, request: &Bound<'_, PyAny>) -> P let pipeline: Bound<'_, PyAny> = compile_audits(request, &invocation)?; let run: AuditRun<'_> = prepare_run(request, &invocation, &pipeline)?; write_headers(&invocation, &pipeline, run.concurrency)?; - let results: Vec> = execute(&invocation, &pipeline, &run)?; + let results = execute(&invocation, &pipeline, &run)?; complete(request, &invocation, &results, &run) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs b/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs index 5e297e4b03..de46d5bee2 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs @@ -189,20 +189,14 @@ pub(crate) fn format_build_execution_json<'py>( } /// The audit-history projection counts, flagged as degraded when any write failed. -fn audit_projection<'py>( +pub(crate) fn audit_projection<'py>( py: Python<'py>, - result: &BuildExecutionResult>, + (attempted, written, failed): (i64, i64, i64), ) -> PyResult)>> { let counts: Vec<(&str, Bound<'py, PyAny>)> = [ - ( - "attempted_count", - result.audit_result_projection_attempted_count, - ), - ( - "written_count", - result.audit_result_projection_written_count, - ), - ("failed_count", result.audit_result_projection_failed_count), + ("attempted_count", attempted), + ("written_count", written), + ("failed_count", failed), ] .into_iter() .map(|(key, value)| Ok((key, any(py, value)?))) @@ -263,8 +257,15 @@ fn degradable_build_json<'py>( result: &BuildExecutionResult>, ) -> PyResult { let execution_succeeded: bool = arguments.flag("execution_succeeded")?; - let rendered: PyResult = audit_projection(py, result) - .and_then(|trailing| build_json(py, arguments, result, trailing)); + let rendered: PyResult = audit_projection( + py, + ( + result.audit_result_projection_attempted_count, + result.audit_result_projection_written_count, + result.audit_result_projection_failed_count, + ), + ) + .and_then(|trailing| build_json(py, arguments, result, trailing)); let error: pyo3::PyErr = match rendered { Ok(payload) => return Ok(payload), Err(error) => error, @@ -601,12 +602,22 @@ pub(crate) fn format_audit_execution_json<'py>( results: Bound<'py, PyAny>, configured_concurrency: Option>, worker_count: Option>, +) -> PyResult { + let all = Vec::>::read(&results)?; + audit_execution_json(py, &all, (configured_concurrency, worker_count), Vec::new()) +} + +/// Audit reporting from already-converted executor results, before the document is serialized. +pub(crate) fn audit_execution_json<'py>( + py: Python<'py>, + all: &[AuditExecutionResult>], + (configured_concurrency, worker_count): (Option>, Option>), + trailing: Vec<(&'static str, Bound<'py, PyAny>)>, ) -> PyResult { let configured_concurrency: Bound<'py, PyAny> = configured_concurrency.map_or_else(|| any(py, 1_i64), Ok)?; let projection: Projection<'py> = Projection::installed(py)?; - let all = Vec::>::read(&results)?; - let results = projection.retained(&all)?; + let results = projection.retained(all)?; let fail_count = results .iter() .filter(|result| result.outcome == AuditOutcome::Error) @@ -655,6 +666,7 @@ pub(crate) fn format_audit_execution_json<'py>( ("worker_count", workers), ], )?), + trailing, ..Extras::default() }, ) diff --git a/fensu.toml b/fensu.toml index 6ccc362a96..e38d23954f 100644 --- a/fensu.toml +++ b/fensu.toml @@ -1672,7 +1672,7 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/analysis_session/session.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/attachments/cursor_intrinsics.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/attachments/reference_markers.rs=3", - "crates/sqlbuild-python/src/bindings/_helpers/audit_command/command.rs=24", + "crates/sqlbuild-python/src/bindings/_helpers/audit_command/command.rs=20", "crates/sqlbuild-python/src/bindings/_helpers/auditing/identity.rs=3", "crates/sqlbuild-python/src/bindings/_helpers/auditing/measurement.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/auditing/schema_audits.rs=1", diff --git a/tests/integration/src/sqlbuild/cli/commands/main/test_audit_command_storage.py b/tests/integration/src/sqlbuild/cli/commands/main/test_audit_command_storage.py index 4f9736e76c..651dff425e 100644 --- a/tests/integration/src/sqlbuild/cli/commands/main/test_audit_command_storage.py +++ b/tests/integration/src/sqlbuild/cli/commands/main/test_audit_command_storage.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json from pathlib import Path import pytest @@ -55,12 +56,29 @@ def test_given_a_target_schema_when_auditing_alone_then_history_is_stored_where_ (tmp_path / "models").mkdir() _ = (tmp_path / "models" / "orders.sql").write_text(_ORDERS_MODEL, encoding="utf-8") assert main(["--no-color", "--project-dir", str(tmp_path), "build"]) == 0 + _ = capsys.readouterr() - exit_code: int = main(["--no-color", "--project-dir", str(tmp_path), "audit"]) + exit_code: int = main(["--no-color", "--project-dir", str(tmp_path), "audit", "--json"]) - output: str = "".join(capsys.readouterr()) + captured: tuple[str, str] = capsys.readouterr() + output: str = "".join(captured) assert exit_code == 0, output assert "warning:" not in output + payload: dict[str, object] = json.loads(captured[0]) + assert payload["command"] == "audit" + assert payload["status"] == "success" + assert payload["summary"] == { + "pass_count": 1, + "warn_count": 0, + "fail_count": 0, + "total_count": 1, + } + assert payload["audit_result_projection"] == { + "attempted_count": 1, + "written_count": 1, + "failed_count": 0, + } + assert "projection_degraded" not in payload db_path: Path = tmp_path / "shop.duckdb" assert query_duckdb_rows( db_path=db_path, From 2588250d49b6732cd44fa3690a7ee3aa7af8338d Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 30 Oct 2026 00:53:00 +0000 Subject: [PATCH 6/8] refactor: keep completed scenario reporting native Read scenario outcomes, comparisons, assertions and nested resource results at the executor edge. Retain typed data through summary, failure and JSON reporting while preserving public projections, sample values and runtime artifact publication. Amp-Thread-ID: https://ampcode.com/threads/T-01a18715-4e00-708e-a87d-6bd109748d66 --- crates/sqlbuild-host/src/models.rs | 68 +++++++ .../bindings/_helpers/native_host/results.rs | 72 ++++++- .../bindings/_helpers/result_output/checks.rs | 176 ++++++++++-------- .../_helpers/result_output/documents.rs | 73 +++++--- .../_helpers/scenario_command/conversions.rs | 154 ++++++++------- .../_helpers/scenario_command/presentation.rs | 7 +- .../_helpers/scenario_command/test_command.rs | 36 ++-- fensu.toml | 2 +- 8 files changed, 384 insertions(+), 204 deletions(-) diff --git a/crates/sqlbuild-host/src/models.rs b/crates/sqlbuild-host/src/models.rs index 42571dc8db..e45c38a8d0 100644 --- a/crates/sqlbuild-host/src/models.rs +++ b/crates/sqlbuild-host/src/models.rs @@ -684,3 +684,71 @@ pub struct BuildExecutionResult { pub audit_result_projection_failed_count: i64, pub timings: BuildExecutionTimings, } + +/// The outcome of a local scenario replay. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ScenarioLocalRunStatus { + Pass, + Fail, + Error, + Skip, +} + +impl ResultEnum for ScenarioLocalRunStatus { + const ALL: &'static [Self] = &[Self::Pass, Self::Fail, Self::Error, Self::Skip]; + + fn value(self) -> &'static str { + match self { + Self::Pass => "PASS", + Self::Fail => "FAIL", + Self::Error => "ERROR", + Self::Skip => "SKIP", + } + } +} + +/// The executor's comparison outcome; absent counts mean the comparison did not run. +#[derive(Clone, Debug, PartialEq)] +pub struct ScenarioExpectedExpectationExecutionResult { + pub model_name: String, + pub status: ExecutionStatus, + pub actual_row_count: Option, + pub expected_row_count: Option, + pub mismatched_row_count: Option, + pub missing_row_count: Option, + pub error_code: Option, + pub error_help: Option, + pub error_message: Option, +} + +/// The executor's zero-row assertion outcome; only warehouse cell values remain host-owned. +#[derive(Clone, Debug, PartialEq)] +pub struct ScenarioAssertionExpectationExecutionResult { + pub name: String, + pub status: ExecutionStatus, + pub failing_row_count: Option, + pub sample_rows: Vec>, + pub error_code: Option, + pub error_help: Option, + pub error_message: Option, +} + +/// The completed scenario fields consumed by command reporting; runtime artifact publication stays at the executor callback edge. +#[derive(Clone, Debug, PartialEq)] +pub struct ScenarioRunResult { + pub scenario_name: String, + pub status: ExecutionStatus, + pub retained: bool, + pub local_status: Option, + pub local_duckdb_path: Option, + pub retained_relations: Option>, + pub seed_results: Vec, + pub function_results: Vec, + pub model_results: Vec>, + pub expected_results: Vec, + pub assertion_results: Vec>, + pub error_code: Option, + pub error_help: Option, + pub error_message: Option, + pub duration_ms: Option, +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/native_host/results.rs b/crates/sqlbuild-python/src/bindings/_helpers/native_host/results.rs index 9686c85cdf..a4149145e5 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/native_host/results.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/native_host/results.rs @@ -6,19 +6,24 @@ use pyo3::exceptions::PyValueError; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult}; use pyo3::types::PyInt; use sqlbuild_core::json::models::JsonInteger; +use sqlbuild_core::scenario_command::models::RetainedRelation; use sqlbuild_host::models::{ AttachedAuditTargetKind, AuditAttachmentKind, AuditEvaluationMode, AuditExecutionResult, AuditOutcome, AuditRunScope, AuditSeverity, BuildExecutionResult, BuildExecutionTimings, BuildStatus, CancellationOutcome, ExecutionPhase, ExecutionResourceKind, ExecutionStatus, FunctionExecutionResult, HookExecutionResult, HookPhase, ItemKind, ItemTimeoutReport, LifeCycleEvent, LifeCycleEventKind, LoadExecutionResult, MeasuredValue, - MicrobatchAccountingInterval, MicrobatchLimitAction, ModelExecutionResult, SeedExecutionResult, - SkipMode, SqlTestColumnDifference, SqlTestDifferenceSample, SqlTestExecutionResult, - SqlTestOutcome, StepResult, + MicrobatchAccountingInterval, MicrobatchLimitAction, ModelExecutionResult, + ScenarioAssertionExpectationExecutionResult, ScenarioExpectedExpectationExecutionResult, + ScenarioLocalRunStatus, ScenarioRunResult, SeedExecutionResult, SkipMode, + SqlTestColumnDifference, SqlTestDifferenceSample, SqlTestExecutionResult, SqlTestOutcome, + StepResult, }; use sqlbuild_host::types::ResultEnum; +use crate::bindings::_helpers::records::construct::enum_from_value; use crate::bindings::_helpers::records::fields::field; +use crate::bindings::_helpers::records::ids::RecordId; /// Extraction at the permanent Python executor edge, including nested result records. pub(crate) trait ReadResult<'py>: Sized { @@ -109,6 +114,7 @@ enums!( ItemKind, CancellationOutcome, MicrobatchLimitAction, + ScenarioLocalRunStatus, ); macro_rules! record { @@ -237,6 +243,66 @@ fn mapping<'py>(value: &Bound<'py, PyAny>) -> PyResult { + name, status, failing_row_count, sample_rows, error_code, error_help, error_message, +}); + +impl<'py> ReadResult<'py> for ScenarioRunResult> { + fn read(value: &Bound<'py, PyAny>) -> PyResult { + let relation_map = field(value, "relation_map")?; + Ok(Self { + scenario_name: ReadResult::read(&field(value, "scenario_name")?)?, + status: ReadResult::read(&field(value, "status")?)?, + retained: ReadResult::read(&field(value, "retained")?)?, + local_status: ReadResult::read(&field(value, "local_status")?)?, + local_duckdb_path: ReadResult::read(&field(value, "local_duckdb_path")?)?, + retained_relations: if relation_map.is_none() { + None + } else { + Some( + field(&relation_map, "artifacts")? + .try_iter()? + .map(|artifact| { + let artifact = artifact?; + let identity = field(&artifact, "identity")?; + let kind = enum_from_value( + value.py(), + RecordId::ScenarioArtifactKind, + field(&identity, "kind")?, + )?; + Ok(RetainedRelation { + kind: field(&kind, "value")?.extract()?, + logical_name: field(&identity, "logical_name")?.extract()?, + physical_name: field(&artifact, "physical_name")?.extract()?, + }) + }) + .collect::>()?, + ) + }, + seed_results: ReadResult::read(&field(value, "seed_results")?)?, + function_results: ReadResult::read(&field(value, "function_results")?)?, + model_results: ReadResult::read(&field(value, "model_results")?)?, + expected_results: ReadResult::read(&field(value, "expected_results")?)?, + assertion_results: ReadResult::read(&field(value, "assertion_results")?)?, + error_code: ReadResult::read(&field(value, "error_code")?)?, + error_help: ReadResult::read(&field(value, "error_help")?)?, + error_message: ReadResult::read(&field(value, "error_message")?)?, + duration_ms: ReadResult::read(&field(value, "duration_ms")?)?, + }) + } +} + impl<'py> ReadResult<'py> for AuditExecutionResult> { fn read(value: &Bound<'py, PyAny>) -> PyResult { let meta = field(value, "audit_meta")?; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs b/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs index ca5b3f660b..0338865724 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/result_output/checks.rs @@ -4,8 +4,8 @@ use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python}; use pyo3::types::{PyDict, PyDictMethods, PyList, PyListMethods, PyTuple}; use sqlbuild_core::json::models::JsonValue; use sqlbuild_host::models::{ - AuditEvaluationMode, AuditExecutionResult, AuditOutcome, MeasuredValue, - SqlTestDifferenceSample, SqlTestExecutionResult, SqlTestOutcome, StepResult, + AuditEvaluationMode, AuditExecutionResult, AuditOutcome, ExecutionStatus, MeasuredValue, + ScenarioRunResult, SqlTestDifferenceSample, SqlTestExecutionResult, SqlTestOutcome, StepResult, }; use sqlbuild_host::types::ResultEnum; @@ -15,17 +15,10 @@ use crate::bindings::_helpers::result_output::assets::{or_none, without_none}; use crate::bindings::_helpers::result_output::projection::{Projection, ResultIdentity, rounded}; use crate::bindings::_helpers::scopes::scope_report_objects::json_to_python; -const SUCCESS_STATUS: &str = "success"; const DECIMAL_KIND: &str = "decimal"; const SCALAR_KINDS: [&str; 5] = ["string", "integer", "boolean", "float", "null"]; const COLLECTION_KINDS: [&str; 2] = ["list", "set"]; const ERROR_FIELDS: [&str; 3] = ["error_code", "error_help", "error_message"]; -const EXPECTED_COUNT_FIELDS: [&str; 4] = [ - "actual_row_count", - "expected_row_count", - "mismatched_row_count", - "missing_row_count", -]; const COMPARISON_COMPLETED: &str = "completed"; const COMPARISON_NOT_RUN: &str = "not_run"; @@ -426,17 +419,23 @@ fn with_errors<'py>( } /// The scenario check with its expected-result and assertion outcomes. -pub(crate) fn scenario_check<'py>(result: &Bound<'py, PyAny>) -> PyResult> { - let py: Python<'py> = result.py(); - let name: String = text(&native_field(result, "scenario_name")?)?; - let status: Bound<'py, PyAny> = value(result, "status")?; +pub(crate) fn scenario_check<'py>( + py: Python<'py>, + result: &ScenarioRunResult>, +) -> PyResult> { + let name = &result.scenario_name; let mut expected: Vec> = Vec::new(); - for item in items(&native_field(result, "expected_results")?)? { - let counts: Vec<(&'static str, Bound<'py, PyAny>)> = fields(&item, &EXPECTED_COUNT_FIELDS)?; - let compared: bool = counts.iter().all(|(_, count)| !count.is_none()); + for item in &result.expected_results { + let counts = [ + ("actual_row_count", item.actual_row_count), + ("expected_row_count", item.expected_row_count), + ("mismatched_row_count", item.mismatched_row_count), + ("missing_row_count", item.missing_row_count), + ]; + let compared = counts.iter().all(|(_, count)| count.is_some()); let mut entries: Vec<(&'static str, Bound<'py, PyAny>)> = vec![ - ("model_name", native_field(&item, "model_name")?), - ("status", value(&item, "status")?), + ("model_name", any(py, &item.model_name)?), + ("status", any(py, item.status.value())?), ( "comparison_status", any( @@ -449,9 +448,14 @@ pub(crate) fn scenario_check<'py>(result: &Bound<'py, PyAny>) -> PyResult = dict(py, entries)?; - for (name, error) in fields(&item, &ERROR_FIELDS)? { + for (name, error) in scenario_errors( + py, + (&item.error_code, &item.error_help, &item.error_message), + )? { if !error.is_none() { result.set_item(name, error)?; } @@ -459,9 +463,8 @@ pub(crate) fn scenario_check<'py>(result: &Bound<'py, PyAny>) -> PyResult> = Vec::new(); - for item in items(&native_field(result, "assertion_results")?)? { - let failing: Bound<'py, PyAny> = native_field(&item, "failing_row_count")?; - let ran: &str = if failing.is_none() { + for item in &result.assertion_results { + let ran: &str = if item.failing_row_count.is_none() { COMPARISON_NOT_RUN } else { COMPARISON_COMPLETED @@ -469,72 +472,95 @@ pub(crate) fn scenario_check<'py>(result: &Bound<'py, PyAny>) -> PyResult = dict( py, vec![ - ("name", native_field(&item, "name")?), - ("status", value(&item, "status")?), + ("name", any(py, &item.name)?), + ("status", any(py, item.status.value())?), ("query_status", any(py, ran)?), - ("failing_row_count", failing), - ("sample_rows", native_field(&item, "sample_rows")?), + ("failing_row_count", any(py, item.failing_row_count)?), + ("sample_rows", any(py, &item.sample_rows)?), ], )?; - for (name, error) in fields(&item, &ERROR_FIELDS)? { + for (name, error) in scenario_errors( + py, + (&item.error_code, &item.error_help, &item.error_message), + )? { if !error.is_none() { result.set_item(name, error)?; } } assertions.push(result); } - with_errors( - result, - vec![ - ("kind", any(py, "scenario")?), - ("name", any(py, name.as_str())?), - ("check_id", any(py, format!("sql_scenario:{name}"))?), - ("passed", any(py, status.eq(SUCCESS_STATUS)?)?), - ("status", status), - ("expected_results", tuple_of(py, expected)?), - ("assertion_results", tuple_of(py, assertions)?), - ], - ) + let mut entries = vec![ + ("kind", any(py, "scenario")?), + ("name", any(py, name.as_str())?), + ("check_id", any(py, format!("sql_scenario:{name}"))?), + ( + "passed", + any(py, result.status == ExecutionStatus::Success)?, + ), + ("status", any(py, result.status.value())?), + ("expected_results", tuple_of(py, expected)?), + ("assertion_results", tuple_of(py, assertions)?), + ]; + entries.extend(scenario_errors( + py, + ( + &result.error_code, + &result.error_help, + &result.error_message, + ), + )?); + without_none(py, entries) } -fn rounded_duration<'py>(result: &Bound<'py, PyAny>) -> PyResult> { - let duration: Bound<'py, PyAny> = native_field(result, "duration_ms")?; - if duration.is_none() { - return Ok(duration); - } - rounded(result.py(), duration.extract()?) +fn scenario_errors<'py>( + py: Python<'py>, + (code, help, message): (&Option, &Option, &Option), +) -> PyResult)>> { + Ok(vec![ + ("error_code", any(py, code)?), + ("error_help", any(py, help)?), + ("error_message", any(py, message)?), + ]) } /// The run outcome of one scenario. -pub(crate) fn scenario_result<'py>(result: &Bound<'py, PyAny>) -> PyResult> { - let py: Python<'py> = result.py(); - let path: Bound<'py, PyAny> = native_field(result, "local_duckdb_path")?; - let local_status: Bound<'py, PyAny> = native_field(result, "local_status")?; - with_errors( - result, - vec![ - ("name", native_field(result, "scenario_name")?), - ("status", value(result, "status")?), - ("duration_ms", rounded_duration(result)?), - ( - "local_status", - if local_status.is_truthy()? { - native_field(&local_status, "value")? - } else { - py.None().into_bound(py) - }, - ), - ("retained", native_field(result, "retained")?), - ( - "local_duckdb_path", - if path.is_truthy()? { - path.str()?.into_any() - } else { - py.None().into_bound(py) - }, - ), - ], - ) +pub(crate) fn scenario_result<'py>( + py: Python<'py>, + result: &ScenarioRunResult>, +) -> PyResult> { + let duration = match result.duration_ms { + Some(duration) => rounded(py, duration)?, + None => py.None().into_bound(py), + }; + let mut entries = vec![ + ("name", any(py, &result.scenario_name)?), + ("status", any(py, result.status.value())?), + ("duration_ms", duration), + ( + "local_status", + any(py, result.local_status.map(|status| status.value()))?, + ), + ("retained", any(py, result.retained)?), + ( + "local_duckdb_path", + any( + py, + result + .local_duckdb_path + .as_ref() + .map(|path| path.as_os_str()), + )?, + ), + ]; + entries.extend(scenario_errors( + py, + ( + &result.error_code, + &result.error_help, + &result.error_message, + ), + )?); + without_none(py, entries) } /// The capture outcome of one scenario snapshot sync or refresh. diff --git a/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs b/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs index de46d5bee2..c6e67673f4 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/result_output/documents.rs @@ -6,7 +6,7 @@ use pyo3::{pyfunction, wrap_pyfunction}; use sqlbuild_host::entries::models::ExecutorHost; use sqlbuild_host::models::{ AuditExecutionResult, AuditOutcome, BuildExecutionResult, BuildStatus, ExecutionStatus, - FunctionExecutionResult, LoadExecutionResult, ModelExecutionResult, SeedExecutionResult, + LoadExecutionResult, ScenarioLocalRunStatus, ScenarioRunResult, SeedExecutionResult, SqlTestExecutionResult, SqlTestOutcome, }; @@ -689,16 +689,14 @@ fn namespace_execution<'py>( ) } -fn scenario_failed(result: &Bound<'_, PyAny>, local: bool) -> PyResult { +fn scenario_failed(result: &ScenarioRunResult>, local: bool) -> bool { if !local { - return native_field(&native_field(result, "status")?, "value")?.eq(FAILED); + return result.status == ExecutionStatus::Failed; } - let local_status: Bound<'_, PyAny> = native_field(result, "local_status")?; - if local_status.is_none() { - return Ok(false); - } - let value: Bound<'_, PyAny> = native_field(&local_status, "value")?; - Ok(value.eq("FAIL")? || value.eq("ERROR")?) + matches!( + result.local_status, + Some(ScenarioLocalRunStatus::Fail | ScenarioLocalRunStatus::Error) + ) } /// Format scenario test command execution results as JSON. @@ -714,48 +712,65 @@ pub(crate) fn format_scenario_execution_json<'py>( options, SCENARIO_OPTIONS, )?; - let results: Bound<'py, PyAny> = arguments.value("results")?; + let results = Vec::>::read(&arguments.value("results")?)?; + scenario_json(py, &arguments, &results) +} + +/// Production scenario completion already owns its converted executor results. +pub(crate) fn format_scenario_execution_json_with<'py>( + py: Python<'py>, + results: &[ScenarioRunResult>], + options: Option>, +) -> PyResult { + let arguments = KeywordArguments::read( + py, + "format_scenario_execution_json", + options, + (&[], SCENARIO_OPTIONS.1), + )?; + scenario_json(py, &arguments, results) +} + +fn scenario_json<'py>( + py: Python<'py>, + arguments: &KeywordArguments<'py>, + results: &[ScenarioRunResult>], +) -> PyResult { let local: bool = arguments.passed("local")? && arguments.flag("local")?; - let run_namespace: Option> = passed(&arguments, "run_namespace")?; - let namespace_source: Option> = passed(&arguments, "namespace_source")?; + let run_namespace: Option> = passed(arguments, "run_namespace")?; + let namespace_source: Option> = passed(arguments, "namespace_source")?; let duration_ms: Option = arguments.value("duration_ms")?.extract()?; let mut projection: Projection<'py> = Projection::installed(py)?; - let mut kept: Vec> = Vec::new(); - for result in items(&results)? { - let name: String = text(&native_field(&result, "scenario_name")?)?; - if projection.resource_terminal_recorded(&name, &format!("sql_scenario:{name}"))? { + let mut kept: Vec<&ScenarioRunResult>> = Vec::new(); + for result in results { + let name = &result.scenario_name; + if projection.resource_terminal_recorded(name, &format!("sql_scenario:{name}"))? { kept.push(result); } } let mut fail_count: usize = 0; let (mut assets, mut checks, mut scenarios) = (Vec::new(), Vec::new(), Vec::new()); for result in &kept { - fail_count += usize::from(scenario_failed(result, local)?); - projection.scenario = Some(text(&native_field(result, "scenario_name")?)?); + fail_count += usize::from(scenario_failed(result, local)); + projection.scenario = Some(result.scenario_name.clone()); assets.extend(seed_assets( &projection, - &Vec::::read(&native_field(result, "seed_results")?)? - .iter() - .collect::>(), + &result.seed_results.iter().collect::>(), None, )?); assets.extend(function_assets( &projection, - &Vec::::read(&native_field(result, "function_results")?)? - .iter() - .collect::>(), + &result.function_results.iter().collect::>(), None, )?); assets.extend(model_assets( &projection, - &Vec::>::read(&native_field(result, "model_results")?)? - .iter() - .collect::>(), + &result.model_results.iter().collect::>(), None, )?); projection.scenario = None; - checks.push(scenario_check(result)?); - scenarios.push(scenario_result(result)?); + checks.push(scenario_check(py, result)?); + scenarios.push(scenario_result(py, result)?); } let mut summary: Bound<'py, PyDict> = summary( py, diff --git a/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/conversions.rs b/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/conversions.rs index d73bd9d710..1263dfe7f2 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/conversions.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/conversions.rs @@ -6,6 +6,7 @@ use std::path::{Path, PathBuf}; use pyo3::exceptions::PyOSError; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python}; +use pyo3::types::PyTuple; use sqlbuild_core::scenario_command::main::build_local_runtime_artifacts::build_local_runtime_artifacts; use sqlbuild_core::scenario_command::main::build_runtime_artifacts::build_runtime_artifacts; use sqlbuild_core::scenario_command::main::write_runtime_artifacts::write_runtime_artifacts; @@ -15,15 +16,14 @@ use sqlbuild_core::scenario_command::models::{ NamedStatements, RetainedInputs, RetainedRelation, ScenarioResultRow, WarehouseRuntimeInputs, }; use sqlbuild_host::entries::models::AdapterHost; +use sqlbuild_host::models::{ExecutionStatus, ScenarioRunResult}; +use sqlbuild_host::types::ResultEnum; use crate::bindings::_helpers::boundary::python_calls::call_with; use crate::bindings::_helpers::native_host::host; -use crate::bindings::_helpers::records::construct::enum_from_value; use crate::bindings::_helpers::records::fields::field as native_field; -use crate::bindings::_helpers::records::ids::RecordId; const SUCCESS_STATUS: &str = "success"; -const FAILED_STATUS: &str = "failed"; const SQL_EVENT_KIND: &str = "sql"; const PASS_STATUS: &str = "PASS"; const FAIL_STATUS: &str = "FAIL"; @@ -66,88 +66,86 @@ fn coded_error(result: &Bound<'_, PyAny>) -> PyResult> { })) } -fn expected_row(expected: &Bound<'_, PyAny>) -> PyResult { - Ok(ExpectedRow { - model_name: native_field(expected, "model_name")?.extract()?, - failed: matching_status(expected, FAILED_STATUS)?, - actual_row_count: native_field(expected, "actual_row_count")?.extract()?, - expected_row_count: native_field(expected, "expected_row_count")?.extract()?, - mismatched_row_count: native_field(expected, "mismatched_row_count")?.extract()?, - missing_row_count: native_field(expected, "missing_row_count")?.extract()?, - error: coded_error(expected)?, +fn result_error( + (code, message, help): (&Option, &Option, &Option), +) -> Option { + message.as_ref().map(|message| CodedText { + code: code.clone(), + message: message.clone(), + help: help.clone(), }) } -fn assertion_row(assertion: &Bound<'_, PyAny>) -> PyResult { - let samples: Vec> = items(&native_field(assertion, "sample_rows")?)?; - Ok(AssertionRow { - name: native_field(assertion, "name")?.extract()?, - failed: matching_status(assertion, FAILED_STATUS)?, - failing_row_count: native_field(assertion, "failing_row_count")?.extract()?, - first_sample: samples - .first() - .map(|sample| sample.str()?.extract()) - .transpose()?, - error: coded_error(assertion)?, - }) -} - -fn retained_relations(relation_map: &Bound<'_, PyAny>) -> PyResult> { - let py: Python<'_> = relation_map.py(); - items(&native_field(relation_map, "artifacts")?)? - .iter() - .map(|artifact| { - let identity: Bound<'_, PyAny> = native_field(artifact, "identity")?; - Ok(RetainedRelation { - kind: enum_value(&enum_from_value( - py, - RecordId::ScenarioArtifactKind, - native_field(&identity, "kind")?, - )?)?, - logical_name: native_field(&identity, "logical_name")?.extract()?, - physical_name: native_field(artifact, "physical_name")?.extract()?, - }) - }) - .collect() -} - /// One `ScenarioRunResult` as its result row shows it. -pub(crate) fn scenario_result_row(result: &Bound<'_, PyAny>) -> PyResult { - let local_status: Bound<'_, PyAny> = native_field(result, "local_status")?; - let local: bool = !local_status.is_none(); - let status: String = if local { - enum_value(&local_status)? - } else if succeeded(result)? { - PASS_STATUS.to_owned() - } else { - FAIL_STATUS.to_owned() - }; - let relation_map: Bound<'_, PyAny> = native_field(result, "relation_map")?; - let duckdb_path: Bound<'_, PyAny> = native_field(result, "local_duckdb_path")?; +pub(crate) fn scenario_result_row( + py: Python<'_>, + result: &ScenarioRunResult>, +) -> PyResult { Ok(ScenarioResultRow { - name: native_field(result, "scenario_name")?.extract()?, - status, - local, - duration_ms: native_field(result, "duration_ms")?.extract()?, - error: coded_error(result)?, - retained: native_field(result, "retained")?.is_truthy()?, - retained_relations: if relation_map.is_none() { - None - } else { - Some(retained_relations(&relation_map)?) - }, - local_duckdb_path: if duckdb_path.is_none() { - None - } else { - Some(posix(&duckdb_path)?) - }, - expected: items(&native_field(result, "expected_results")?)? + name: result.scenario_name.clone(), + status: result + .local_status + .map_or_else( + || { + if result.status == ExecutionStatus::Success { + PASS_STATUS + } else { + FAIL_STATUS + } + }, + |status| status.value(), + ) + .to_owned(), + local: result.local_status.is_some(), + duration_ms: result.duration_ms, + error: result_error(( + &result.error_code, + &result.error_message, + &result.error_help, + )), + retained: result.retained, + retained_relations: result.retained_relations.clone(), + local_duckdb_path: result.local_duckdb_path.as_ref().map(|path| { + path.to_string_lossy() + .replace(std::path::MAIN_SEPARATOR, "/") + }), + expected: result + .expected_results .iter() - .map(expected_row) - .collect::>()?, - assertions: items(&native_field(result, "assertion_results")?)? + .map(|expected| ExpectedRow { + model_name: expected.model_name.clone(), + failed: expected.status == ExecutionStatus::Failed, + actual_row_count: expected.actual_row_count, + expected_row_count: expected.expected_row_count, + mismatched_row_count: expected.mismatched_row_count, + missing_row_count: expected.missing_row_count, + error: result_error(( + &expected.error_code, + &expected.error_message, + &expected.error_help, + )), + }) + .collect(), + assertions: result + .assertion_results .iter() - .map(assertion_row) + .map(|assertion| { + let first_sample = match assertion.sample_rows.first() { + Some(row) => Some(PyTuple::new(py, row)?.str()?.extract()?), + None => None, + }; + Ok(AssertionRow { + name: assertion.name.clone(), + failed: assertion.status == ExecutionStatus::Failed, + failing_row_count: assertion.failing_row_count, + first_sample, + error: result_error(( + &assertion.error_code, + &assertion.error_message, + &assertion.error_help, + )), + }) + }) .collect::>()?, }) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/presentation.rs b/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/presentation.rs index 1de783b86a..e9b31ff951 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/presentation.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/presentation.rs @@ -12,12 +12,14 @@ use sqlbuild_core::scenario_command::main::render_snapshot_sync_result::render_s use sqlbuild_core::scenario_command::main::scenario_activity_message::scenario_activity_message; use sqlbuild_core::scenario_command::models::ScenarioNamespace; use sqlbuild_host::entries::models::ExecutorHost; +use sqlbuild_host::models::ScenarioRunResult; use crate::bindings::_helpers::boundary::python_calls::call_with; use crate::bindings::_helpers::boundary::python_calls::keywords; use crate::bindings::_helpers::build_progress::progress_reporters::ConnectionProgressReporter; use crate::bindings::_helpers::build_progress::transient_status::status_reporter; use crate::bindings::_helpers::native_host::host; +use crate::bindings::_helpers::native_host::results::ReadResult; use crate::bindings::_helpers::records::fields::field as native_field; use crate::bindings::_helpers::scenario_command::conversions::{ capture_row, scenario_result_row, write_local_runtime_target, write_warehouse_runtime_target, @@ -86,7 +88,10 @@ impl ScenarioProgressRelay { write_local_runtime_target(target_dir, (adapter, plan, &result))?; } } - render_scenario_result(&scenario_result_row(&result)?, style) + render_scenario_result( + &scenario_result_row(py, &ScenarioRunResult::read(&result)?)?, + style, + ) } CompletionKind::Capture => { render_capture_result(&capture_row(&result, (project_dir, plan))?, style) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/test_command.rs b/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/test_command.rs index 0d0caf736a..03b9b01f0a 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/test_command.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/scenario_command/test_command.rs @@ -15,6 +15,8 @@ use sqlbuild_core::scenario_command::models::{ ScenarioNamespace, ScenarioResultRow, ScenarioTestFlags, SqlAnalysisCommand, }; use sqlbuild_host::entries::models::ExecutorHost; +use sqlbuild_host::models::{ExecutionStatus, ScenarioRunResult}; +use sqlbuild_host::types::ResultEnum; use crate::bindings::_helpers::boundary::python_calls::call_with; use crate::bindings::_helpers::boundary::python_calls::keywords; @@ -26,10 +28,11 @@ use crate::bindings::_helpers::change_selection::selection::{ }; use crate::bindings::_helpers::command_runtime::concurrency::resolve_concurrency; use crate::bindings::_helpers::native_host::host; +use crate::bindings::_helpers::native_host::results::ReadResult; use crate::bindings::_helpers::records::fields::field as native_field; -use crate::bindings::_helpers::result_output::documents::format_scenario_execution_json; +use crate::bindings::_helpers::result_output::documents::format_scenario_execution_json_with; use crate::bindings::_helpers::result_output::documents::write_execution_json_output; -use crate::bindings::_helpers::scenario_command::conversions::{scenario_result_row, succeeded}; +use crate::bindings::_helpers::scenario_command::conversions::scenario_result_row; use crate::bindings::_helpers::scenario_command::presentation::{ CompletionKind, RelayContext, SectionStatus, activity, begin_section, connection_hooks, progress_relay, write_completion, @@ -140,7 +143,11 @@ fn prepare<'py>(py: Python<'py>, request: &Bound<'py, PyAny>) -> PyResult, run: &TestRun<'_>, - (section, started, results): (&SectionStatus<'_>, Instant, &Bound<'_, PyTuple>), + (section, started, results): ( + &SectionStatus<'_>, + Instant, + &[ScenarioRunResult>], + ), (counts, succeeded): (Vec, bool), ) -> PyResult<()> { let py: Python<'_> = request.py(); @@ -155,7 +162,7 @@ fn finish( )?; let rows: Vec = results .iter() - .map(|result| scenario_result_row(&result)) + .map(|result| scenario_result_row(py, result)) .collect::>()?; let failures: String = render_scenario_failures(&rows, run.style); if !failures.is_empty() { @@ -167,12 +174,12 @@ fn finish( let payload: Bound<'_, PyAny> = PyString::new( py, &with_since_section( - format_scenario_execution_json( + format_scenario_execution_json_with( py, + results, Some(keywords( py, &[ - ("results", results.clone().into_any()), ("local", boolean(py, run.local)), ( "run_namespace", @@ -302,21 +309,16 @@ fn run_scenarios( ]); ExecutorHost::RunScenarioTestPipeline }; - let results: Bound<'_, PyTuple> = PyTuple::new( - py, - call_with(&host::entry(py, runner)?, &arguments)? - .try_iter()? - .collect::>>()?, - )?; + let results = + Vec::>::read(&call_with(&host::entry(py, runner)?, &arguments)?)?; let total: usize = results.len(); let (counts, ok): (Vec, bool) = if run.local { let mut tallies: [usize; 4] = [0; 4]; for result in results.iter() { - let status: Bound<'_, PyAny> = result.getattr("local_status")?; - if status.is_none() { + let Some(status) = result.local_status else { continue; - } - let value: String = status.getattr("value")?.extract()?; + }; + let value = status.value(); if let Some(index) = LOCAL_STATUSES.iter().position(|label| *label == value) { tallies[index] += 1; } @@ -331,7 +333,7 @@ fn run_scenarios( } else { let mut passed: usize = 0; for result in results.iter() { - if succeeded(&result)? { + if result.status == ExecutionStatus::Success { passed += 1; } } diff --git a/fensu.toml b/fensu.toml index e38d23954f..72dbffe8d4 100644 --- a/fensu.toml +++ b/fensu.toml @@ -2201,7 +2201,7 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/scenario_command/presentation.rs=5", "crates/sqlbuild-python/src/bindings/_helpers/scenario_command/setup.rs=14", "crates/sqlbuild-python/src/bindings/_helpers/scenario_command/snapshot_sync.rs=2", - "crates/sqlbuild-python/src/bindings/_helpers/scenario_command/test_command.rs=15", + "crates/sqlbuild-python/src/bindings/_helpers/scenario_command/test_command.rs=13", "crates/sqlbuild-python/src/bindings/_helpers/scopes/compile_usages.rs=4", "crates/sqlbuild-python/src/bindings/_helpers/scopes/declaration_contexts.rs=6", "crates/sqlbuild-python/src/bindings/_helpers/scopes/declaration_resolution.rs=15", From 97a274b79d9d02c0b13c002d6484b8601a390024 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 30 Oct 2026 01:25:35 +0000 Subject: [PATCH 7/8] refactor: keep SQLite history locking native and interruptible Preserve reentrant callbacks and check Python signals during contended native waits. Retain connection hooks and transaction recovery, with a real SQLite cancellation regression. Amp-Thread-ID: https://ampcode.com/threads/T-01a18715-4e00-708e-a87d-6bd109748d66 --- .../_helpers/execution_history/sqlite.rs | 20 ++--- .../_helpers/execution_history/statements.rs | 13 --- .../bindings/_helpers/native_threads/mod.rs | 1 + .../_helpers/native_threads/reentrant_lock.rs | 71 ++++++++++++++++ fensu.toml | 5 +- .../runtime/execution_history/_test_types.py | 6 ++ .../test_sqlite_execution_history.py | 83 +++++++++++++++++++ 7 files changed, 169 insertions(+), 30 deletions(-) create mode 100644 crates/sqlbuild-python/src/bindings/_helpers/native_threads/reentrant_lock.rs diff --git a/crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs b/crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs index 387a1cee99..67818830ab 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs @@ -7,7 +7,7 @@ use std::sync::OnceLock; use std::sync::atomic::{AtomicBool, AtomicI64, Ordering}; use pyo3::IntoPyObjectExt; -use pyo3::prelude::{Bound, Py, PyAny, PyAnyMethods, PyErr, PyResult, Python, pyclass, pymethods}; +use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyErr, PyResult, Python, pyclass, pymethods}; use pyo3::types::{PyDict, PyTuple}; use sqlbuild_host::entries::models::ExecutorHost; use sqlbuild_state::execution_history::constants::{ @@ -50,14 +50,14 @@ use crate::bindings::_helpers::execution_history::conversion::{ text_of, utc_time, }; use crate::bindings::_helpers::execution_history::statements::{ - column, execute, execute_plain, execute_query, fetch_one, locked, tuple_of, + column, execute, execute_plain, execute_query, fetch_one, tuple_of, }; use crate::bindings::_helpers::native_host::host; +use crate::bindings::_helpers::native_threads::reentrant_lock::ReentrantLock; use crate::bindings::_helpers::records::fields::field as native_field; const SQLITE_MODULE: &str = "sqlite3"; const PATHLIB_MODULE: &str = "pathlib"; -const THREADING_MODULE: &str = "threading"; const MILLIS_PER_SECOND: f64 = 1000.0; const CLOSED: &str = "SQLite execution history storage is closed"; const NOT_AWARE: &str = "SQLite history timestamp is not timezone-aware"; @@ -188,7 +188,7 @@ fn stored_run(row: &Bound<'_, PyAny>) -> PyResult { )] pub(crate) struct SqliteExecutionHistory { path: OnceLock, - lock: OnceLock>, + lock: ReentrantLock, closed: AtomicBool, project_calls: AtomicI64, } @@ -203,10 +203,7 @@ impl SqliteExecutionHistory { py: Python<'py>, body: impl FnOnce() -> PyResult, ) -> PyResult { - let Some(lock) = self.lock.get() else { - return Err(storage_error(py, CLOSED)); - }; - locked(lock.bind(py), body) + self.lock.with_lock(py, body) } fn ensure_open(&self, py: Python<'_>) -> PyResult<()> { @@ -453,7 +450,7 @@ impl SqliteExecutionHistory { fn new(_args: &Bound<'_, PyTuple>, _kwargs: Option<&Bound<'_, PyDict>>) -> Self { Self { path: OnceLock::new(), - lock: OnceLock::new(), + lock: ReentrantLock::default(), closed: AtomicBool::new(true), project_calls: AtomicI64::new(0), } @@ -490,8 +487,6 @@ impl SqliteExecutionHistory { let database: String = checked(py, sqlite_database_path(&location))?; let busy_timeout_ms: i64 = location.busy_timeout_ms.unwrap_or(SQLITE_BUSY_TIMEOUT_MS); let _ = this.path.set(database.clone()); - let lock: Bound<'_, PyAny> = imported(py, THREADING_MODULE, "RLock")?.call0()?; - let _ = this.lock.set(lock.unbind()); this.closed.store(false, Ordering::SeqCst); let mut connection: Option> = None; let opened: PyResult<()> = (|| { @@ -842,9 +837,6 @@ impl SqliteExecutionHistory { fn close(slf: &Bound<'_, Self>) -> PyResult<()> { let py: Python<'_> = slf.py(); let this: &Self = slf.get(); - if this.lock.get().is_none() { - return Ok(()); - } this.with_lock(py, || { if this.closed.load(Ordering::SeqCst) { return Ok(()); diff --git a/crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs b/crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs index 54e2c571f4..8a63f38744 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs @@ -43,19 +43,6 @@ pub(crate) fn fetch_all<'py>(cursor: &Bound<'py, PyAny>) -> PyResult( - lock: &Bound<'_, PyAny>, - body: impl FnOnce() -> PyResult, -) -> PyResult { - lock.call_method0("acquire")?; - let result: PyResult = body(); - let released = lock.call_method0("release"); - let value: T = result?; - released?; - Ok(value) -} - /// `row[name]`. pub(crate) fn column<'py>(row: &Bound<'py, PyAny>, name: &str) -> PyResult> { row.get_item(name) diff --git a/crates/sqlbuild-python/src/bindings/_helpers/native_threads/mod.rs b/crates/sqlbuild-python/src/bindings/_helpers/native_threads/mod.rs index c379120844..b98894c8c2 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/native_threads/mod.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/native_threads/mod.rs @@ -1,3 +1,4 @@ pub(crate) mod constants; pub(crate) mod exit_signal; pub(crate) mod interruptible_join; +pub(crate) mod reentrant_lock; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/native_threads/reentrant_lock.rs b/crates/sqlbuild-python/src/bindings/_helpers/native_threads/reentrant_lock.rs new file mode 100644 index 0000000000..44c5319046 --- /dev/null +++ b/crates/sqlbuild-python/src/bindings/_helpers/native_threads/reentrant_lock.rs @@ -0,0 +1,71 @@ +//! A native reentrant lock whose contended waits release the GIL and honor Python signals. + +use std::sync::{Condvar, Mutex, PoisonError}; +use std::thread::{ThreadId, current}; +use std::time::Duration; + +use pyo3::exceptions::PyRuntimeError; +use pyo3::prelude::{PyResult, Python}; + +const SIGNAL_POLL_INTERVAL: Duration = Duration::from_millis(5); +const POISONED: &str = "native reentrant lock is poisoned"; + +#[derive(Default)] +struct LockState { + owner: Option, + depth: usize, +} + +/// Serialize native operations that may reenter through Python callbacks. +#[derive(Default)] +pub(crate) struct ReentrantLock { + state: Mutex, + available: Condvar, +} + +/// Release one acquisition even when the protected callback raises or panics. +struct LockGuard<'a>(&'a ReentrantLock); + +impl ReentrantLock { + /// Run `body` under the lock, checking pending Python signals while another thread owns it. + pub(crate) fn with_lock( + &self, + py: Python<'_>, + body: impl FnOnce() -> PyResult, + ) -> PyResult { + while !py + .detach(|| self.acquire_or_wait()) + .map_err(PyRuntimeError::new_err)? + { + py.check_signals()?; + } + let _guard = LockGuard(self); + body() + } + + fn acquire_or_wait(&self) -> Result { + let owner: ThreadId = current().id(); + let mut state = self.state.lock().map_err(|_| POISONED)?; + if state.owner.is_none() || state.owner == Some(owner) { + state.owner = Some(owner); + state.depth += 1; + return Ok(true); + } + let (_state, _) = self + .available + .wait_timeout(state, SIGNAL_POLL_INTERVAL) + .map_err(|_| POISONED)?; + Ok(false) + } +} + +impl Drop for LockGuard<'_> { + fn drop(&mut self) { + let mut state = self.0.state.lock().unwrap_or_else(PoisonError::into_inner); + state.depth -= 1; + if state.depth == 0 { + state.owner = None; + self.0.available.notify_one(); + } + } +} diff --git a/fensu.toml b/fensu.toml index 72dbffe8d4..d20d861f41 100644 --- a/fensu.toml +++ b/fensu.toml @@ -1893,8 +1893,8 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/execution_history/conversion.rs=19", "crates/sqlbuild-python/src/bindings/_helpers/execution_history/functions.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/execution_history/postgres.rs=28", - "crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs=35", - "crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs=6", + "crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs=33", + "crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs=4", "crates/sqlbuild-python/src/bindings/_helpers/exported_constants/registration.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/fingerprints/latest_rows.rs=16", "crates/sqlbuild-python/src/bindings/_helpers/fingerprints/query_hashes.rs=1", @@ -2369,7 +2369,6 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/event_exporting/models.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/event_exporting/scope.rs=9", "crates/sqlbuild-python/src/bindings/_helpers/event_exporting/workers.rs=2", - "crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/fingerprints/storage_io.rs=12", "crates/sqlbuild-python/src/bindings/_helpers/format_command/command.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/format_command/files.rs=1", diff --git a/tests/integration/src/sqlbuild/runtime/execution_history/_test_types.py b/tests/integration/src/sqlbuild/runtime/execution_history/_test_types.py index 6cf21fb478..60fe069a8a 100644 --- a/tests/integration/src/sqlbuild/runtime/execution_history/_test_types.py +++ b/tests/integration/src/sqlbuild/runtime/execution_history/_test_types.py @@ -37,6 +37,12 @@ class SQLiteTransactionFailureCase: expected_run_count: int +@dataclass(frozen=True) +class SQLiteInterruptCase: + description: str + expected_stdout: str + + @dataclass(frozen=True) class PostgresHistoryCase: description: str diff --git a/tests/integration/src/sqlbuild/runtime/execution_history/test_sqlite_execution_history.py b/tests/integration/src/sqlbuild/runtime/execution_history/test_sqlite_execution_history.py index 765a65665a..afc6d03d9d 100644 --- a/tests/integration/src/sqlbuild/runtime/execution_history/test_sqlite_execution_history.py +++ b/tests/integration/src/sqlbuild/runtime/execution_history/test_sqlite_execution_history.py @@ -1,10 +1,13 @@ """SQLite migration, reopen, reconciliation, and atomicity integration tests.""" import sqlite3 +import subprocess +import sys from collections.abc import Callable from concurrent.futures import Future, ThreadPoolExecutor, TimeoutError from datetime import UTC, datetime from pathlib import Path +from textwrap import dedent from threading import Event from typing import Any, cast from unittest.mock import Mock @@ -27,6 +30,7 @@ from sqlbuild.observability import LifecycleEvent, OpaqueLifecycleEvent from sqlbuild.sqlite_history import SQLiteExecutionHistory from tests.integration.src.sqlbuild.runtime.execution_history._test_types import ( + SQLiteInterruptCase, SQLiteMigrationCase, SQLitePathCase, SQLitePersistenceCase, @@ -627,3 +631,82 @@ def tracked_connect( opened[0].execute("SELECT 1") assert len(opened) == test_case.expected_event_count + 1 assert test_case.expected_run_count == 0 + + +@pytest.mark.skipif(sys.platform == "win32", reason="subprocess SIGINT delivery requires POSIX") +@pytest.mark.parametrize( + "test_case", + ( + SQLiteInterruptCase( + description="interrupt remains observable while another thread owns history", + expected_stdout="interrupted before owner release; storage closed cleanly\n", + ), + ), + ids=lambda case: case.description, +) +def test_given_contended_history_when_interrupted_then_interrupt_precedes_owner_release( + test_case: SQLiteInterruptCase, +) -> None: + script: str = dedent( + """ + import os + import signal + import threading + + from sqlbuild.execution_history import EventFilter + from sqlbuild.sqlite_history import SQLiteExecutionHistory + + signal.signal(signal.SIGINT, signal.default_int_handler) + + class BlockingHistory(SQLiteExecutionHistory): + def __init__(self): + self.block = False + self.entered = threading.Event() + self.release = threading.Event() + super().__init__(path=":memory:") + + def _read_all_events(self): + rows = super()._read_all_events() + if self.block: + self.entered.set() + assert self.release.wait(10) + return rows + + history = BlockingHistory() + history.block = True + owner = threading.Thread(target=history.reconcile) + owner.start() + assert history.entered.wait(5) + interrupt = threading.Timer(0.25, lambda: os.kill(os.getpid(), signal.SIGINT)) + fallback = threading.Timer(3, history.release.set) + interrupt.start() + fallback.start() + try: + try: + history.get_events(event_filter=EventFilter()) + except KeyboardInterrupt: + assert not history.release.is_set(), "interrupt waited for the owner" + else: + raise AssertionError("expected KeyboardInterrupt") + finally: + history.release.set() + interrupt.cancel() + fallback.cancel() + owner.join(5) + assert not owner.is_alive() + history.close() + print("interrupted before owner release; storage closed cleanly") + """ + ) + completed: subprocess.CompletedProcess[str] = subprocess.run( + [sys.executable, "-c", script], + capture_output=True, + text=True, + timeout=15, + check=False, + ) + + assert (completed.returncode, completed.stdout) == ( + 0, + test_case.expected_stdout, + ), completed.stderr From 6a815362468ed3bd72840fbfc5bc96192e2d52b3 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 30 Oct 2026 01:51:54 +0000 Subject: [PATCH 8/8] refactor: use native SQLite history connections Keep local history connections and query rows native while preserving the public storage API, connection overrides, transaction recovery and interruptible locking. Preserve literal filename semantics even when bundled SQLite enables URI interpretation globally. Cover persistence without Python connect and exact relative filenames through the public API. Amp-Thread-ID: T-01a18715-4e00-708e-a87d-6bd109748d66 --- Cargo.lock | 1 + crates/sqlbuild-python/Cargo.toml | 1 + .../_helpers/execution_history/sqlite.rs | 50 ++--- .../_helpers/execution_history/statements.rs | 24 +++ .../src/bindings/_helpers/native_host/mod.rs | 4 +- .../_helpers/native_host/sqlite_connection.rs | 189 ++++++++++++++++++ .../_helpers/native_host/sqlite_cursor.rs | 99 +++++++++ fensu.toml | 2 +- src/sqlbuild/_native.pyi | 3 +- .../test_sqlite_execution_history.py | 90 ++++++--- .../execution_history/conformance/conftest.py | 2 +- 11 files changed, 414 insertions(+), 51 deletions(-) create mode 100644 crates/sqlbuild-python/src/bindings/_helpers/native_host/sqlite_connection.rs create mode 100644 crates/sqlbuild-python/src/bindings/_helpers/native_host/sqlite_cursor.rs diff --git a/Cargo.lock b/Cargo.lock index c707638f60..f909a3f395 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1371,6 +1371,7 @@ dependencies = [ "mimalloc", "pyo3", "rayon", + "rusqlite", "rustc-hash", "serde", "serde_json", diff --git a/crates/sqlbuild-python/Cargo.toml b/crates/sqlbuild-python/Cargo.toml index 2ffda3e998..661bd3d6a1 100644 --- a/crates/sqlbuild-python/Cargo.toml +++ b/crates/sqlbuild-python/Cargo.toml @@ -20,6 +20,7 @@ fensu-policy.workspace = true mimalloc.workspace = true pyo3.workspace = true rayon.workspace = true +rusqlite.workspace = true rustc-hash.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs b/crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs index 67818830ab..ea38f66ca7 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs @@ -41,7 +41,6 @@ use sqlbuild_state::execution_history::models::{ use sqlbuild_state::execution_history::types::UtcTime; use crate::bindings::_helpers::boundary::python_calls::imported; -use crate::bindings::_helpers::boundary::python_calls::keywords; use crate::bindings::_helpers::execution_history::appends::prepared_batch; use crate::bindings::_helpers::execution_history::conversion::{ checked, event_filter_spec, exact_int, history_error_of_kind, new_namespace, now, @@ -53,12 +52,13 @@ use crate::bindings::_helpers::execution_history::statements::{ column, execute, execute_plain, execute_query, fetch_one, tuple_of, }; use crate::bindings::_helpers::native_host::host; +use crate::bindings::_helpers::native_host::sqlite_connection::{SqliteConnection, sqlite_params}; +use crate::bindings::_helpers::native_host::sqlite_cursor::SqliteCursor; use crate::bindings::_helpers::native_threads::reentrant_lock::ReentrantLock; use crate::bindings::_helpers::records::fields::field as native_field; const SQLITE_MODULE: &str = "sqlite3"; const PATHLIB_MODULE: &str = "pathlib"; -const MILLIS_PER_SECOND: f64 = 1000.0; const CLOSED: &str = "SQLite execution history storage is closed"; const NOT_AWARE: &str = "SQLite history timestamp is not timezone-aware"; const INTEGRITY_CONFLICT: ExecutorHost = ExecutorHost::IntegrityConflictError; @@ -233,7 +233,10 @@ impl SqliteExecutionHistory { }; if let Err(error) = execute_plain(&connection, SQLITE_COMMIT) { if sqlite_failure(py, &error)? - && native_field(&connection, "in_transaction")?.is_truthy()? + && match connection.cast::() { + Ok(connection) => connection.get().in_transaction(py)?, + Err(_) => native_field(&connection, "in_transaction")?.is_truthy()?, + } { let _ = execute_plain(&connection, SQLITE_ROLLBACK); } @@ -310,6 +313,16 @@ impl SqliteExecutionHistory { fn replace_projection(connection: &Bound<'_, PyAny>, projected: &[RunRow]) -> PyResult<()> { let py: Python<'_> = connection.py(); execute_plain(connection, SQLITE_DELETE_RUNS)?; + if let Ok(connection) = connection.cast::() { + for run in projected { + connection.get().execute_native( + py, + SQLITE_RUN_INSERT, + &sqlite_params(&run_insert_query(Dialect::Sqlite, run).params)?, + )?; + } + return Ok(()); + } let rows: Vec> = projected .iter() .map(|run| { @@ -347,10 +360,16 @@ impl SqliteExecutionHistory { None => { let query = event_insert_query(Dialect::Sqlite, event, &now(py)?); let cursor: Bound<'py, PyAny> = execute_query(connection, &query)?; - let storage_id: Bound<'py, PyAny> = native_field(&cursor, "lastrowid")?; - connection - .call_method1("execute", (SQLITE_EVENT_BY_STORAGE_ID, (storage_id,)))? - .call_method0("fetchone")? + let storage_id: i64 = match cursor.cast::() { + Ok(cursor) => cursor.get().last_row_id, + Err(_) => native_field(&cursor, "lastrowid")?.extract()?, + }; + fetch_one(&execute( + connection, + SQLITE_EVENT_BY_STORAGE_ID, + &[SqlParam::Integer(storage_id)], + )?)? + .unwrap_or_else(|| py.None().into_bound(py)) } }; if namespace.is_none() { @@ -491,25 +510,10 @@ impl SqliteExecutionHistory { let mut connection: Option> = None; let opened: PyResult<()> = (|| { prepare_history_directory(&database)?; - let sqlite = py.import(SQLITE_MODULE)?; - let opened: Bound<'_, PyAny> = native_field(&sqlite, "connect")?.call( - (database.as_str(),), - Some(&keywords( - py, - &[ - ( - "timeout", - (busy_timeout_ms as f64 / MILLIS_PER_SECOND).into_bound_py_any(py)?, - ), - ("isolation_level", py.None().into_bound(py)), - ("check_same_thread", false.into_bound_py_any(py)?), - ], - )?), - )?; + let opened = Bound::new(py, SqliteConnection::open(py, &database)?)?.into_any(); connection = Some(opened.clone()); slf.setattr("_connection", &opened)?; this.with_lock(py, || { - opened.setattr("row_factory", native_field(&sqlite, "Row")?)?; execute_plain(&opened, &format!("PRAGMA busy_timeout = {busy_timeout_ms}"))?; execute_plain(&opened, SQLITE_FOREIGN_KEYS)?; if database != MEMORY_PATH { diff --git a/crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs b/crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs index 8a63f38744..cbb9f83ccf 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs @@ -5,6 +5,8 @@ use pyo3::types::PyTuple; use sqlbuild_state::execution_history::models::{Query, SqlParam}; use crate::bindings::_helpers::execution_history::conversion::python_params; +use crate::bindings::_helpers::native_host::sqlite_connection::{SqliteConnection, sqlite_params}; +use crate::bindings::_helpers::native_host::sqlite_cursor::{SqliteCursor, SqliteRow}; /// `target.execute(sql, params)`. pub(crate) fn execute<'py>( @@ -13,6 +15,15 @@ pub(crate) fn execute<'py>( params: &[SqlParam], ) -> PyResult> { let py: Python<'py> = target.py(); + if let Ok(connection) = target.cast::() { + return Bound::new( + py, + connection + .get() + .execute_native(py, sql, &sqlite_params(params)?)?, + ) + .map(Bound::into_any); + } target.call_method1("execute", (sql, python_params(py, params)?)) } @@ -21,6 +32,9 @@ pub(crate) fn execute_plain<'py>( target: &Bound<'py, PyAny>, sql: &str, ) -> PyResult> { + if target.is_instance_of::() { + return execute(target, sql, &[]); + } target.call_method1("execute", (sql,)) } @@ -34,6 +48,13 @@ pub(crate) fn execute_query<'py>( /// The first row `cursor.fetchone()` returns, or `None`. pub(crate) fn fetch_one<'py>(cursor: &Bound<'py, PyAny>) -> PyResult>> { + if let Ok(cursor) = cursor.cast::() { + return cursor + .get() + .next_row() + .map(|row| Bound::new(cursor.py(), row).map(Bound::into_any)) + .transpose(); + } let row: Bound<'py, PyAny> = cursor.call_method0("fetchone")?; Ok((!row.is_none()).then_some(row)) } @@ -45,6 +66,9 @@ pub(crate) fn fetch_all<'py>(cursor: &Bound<'py, PyAny>) -> PyResult(row: &Bound<'py, PyAny>, name: &str) -> PyResult> { + if let Ok(row) = row.cast::() { + return row.get().column(row.py(), name); + } row.get_item(name) } diff --git a/crates/sqlbuild-python/src/bindings/_helpers/native_host/mod.rs b/crates/sqlbuild-python/src/bindings/_helpers/native_host/mod.rs index f06d515bb8..4737c79a25 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/native_host/mod.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/native_host/mod.rs @@ -1,6 +1,8 @@ -//! The one Python module native code imports by path, and the staying-Python entries it names. +//! Permanent Python host calls and compatibility views of native-owned execution data. pub(crate) mod connections; pub(crate) mod host; pub(crate) mod registration; pub(crate) mod results; +pub(crate) mod sqlite_connection; +pub(crate) mod sqlite_cursor; diff --git a/crates/sqlbuild-python/src/bindings/_helpers/native_host/sqlite_connection.rs b/crates/sqlbuild-python/src/bindings/_helpers/native_host/sqlite_connection.rs new file mode 100644 index 0000000000..c82f67e182 --- /dev/null +++ b/crates/sqlbuild-python/src/bindings/_helpers/native_host/sqlite_connection.rs @@ -0,0 +1,189 @@ +//! Rust-owned SQLite connection; Python views exist only for connection overrides. + +use std::collections::VecDeque; +use std::path::PathBuf; +use std::sync::{Arc, Mutex, PoisonError}; + +use pyo3::exceptions::PyValueError; +use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyErr, PyResult, Python, pyclass, pymethods}; +use rusqlite::types::Value; +use rusqlite::{Connection, Error, ErrorCode, OpenFlags, params_from_iter}; +use sqlbuild_state::execution_history::constants::MEMORY_PATH; +use sqlbuild_state::execution_history::models::SqlParam; + +use crate::bindings::_helpers::boundary::python_calls::imported; +use crate::bindings::_helpers::native_host::sqlite_cursor::{SqliteCursor, SqliteRow}; + +/// Preserve DB-API exception classes at the public storage-error boundary. +fn database_error(py: Python<'_>, error: Error) -> PyErr { + let class: &str = match &error { + Error::SqliteFailure(error, _) if error.code == ErrorCode::ConstraintViolation => { + "IntegrityError" + } + Error::SqliteFailure(error, _) + if matches!( + error.code, + ErrorCode::DatabaseCorrupt | ErrorCode::NotADatabase + ) => + { + "DatabaseError" + } + _ => "OperationalError", + }; + match imported(py, "sqlite3", class).and_then(|class| class.call1((error.to_string(),))) { + Ok(error) => PyErr::from_value(error), + Err(error) => error, + } +} + +/// Parameters of history SQL contain no Python-owned values. +pub(crate) fn sqlite_params(params: &[SqlParam]) -> PyResult> { + params + .iter() + .map(|value| { + Ok(match value { + SqlParam::Null => Value::Null, + SqlParam::Text(value) => Value::Text(value.clone()), + SqlParam::Integer(value) => Value::Integer(*value), + SqlParam::Flag(value) => Value::Integer(i64::from(*value)), + SqlParam::Timestamp(_) | SqlParam::TextList(_) => { + return Err(PyValueError::new_err("Postgres-only history parameter")); + } + }) + }) + .collect() +} + +#[pyclass(frozen, module = "sqlbuild._native", name = "_HistorySqliteConnection")] +pub(crate) struct SqliteConnection { + connection: Mutex>, +} + +impl SqliteConnection { + /// Open literal filenames, using absolute paths because bundled SQLite enables URI handling globally. + pub(crate) fn open(py: Python<'_>, path: &str) -> PyResult { + let flags = OpenFlags::default() & !OpenFlags::SQLITE_OPEN_URI; + let database: PathBuf = if path == MEMORY_PATH { + PathBuf::from(path) + } else { + std::env::current_dir()?.join(path) + }; + let connection = py + .detach(|| Connection::open_with_flags(database, flags)) + .map_err(|error| database_error(py, error))?; + Ok(Self { + connection: Mutex::new(Some(connection)), + }) + } + + fn with_connection( + &self, + py: Python<'_>, + body: impl FnOnce(&Connection) -> rusqlite::Result + Send, + ) -> PyResult { + let result = py.detach(|| { + let guard = self + .connection + .lock() + .unwrap_or_else(PoisonError::into_inner); + guard.as_ref().map(body) + }); + match result { + Some(result) => result.map_err(|error| database_error(py, error)), + None => Err(PyErr::from_value( + imported(py, "sqlite3", "ProgrammingError")? + .call1(("Cannot operate on a closed database.",))?, + )), + } + } + + pub(crate) fn execute_native( + &self, + py: Python<'_>, + sql: &str, + params: &[Value], + ) -> PyResult { + self.with_connection(py, |connection| { + let mut statement = connection.prepare_cached(sql)?; + let columns = Arc::new( + statement + .column_names() + .into_iter() + .map(str::to_owned) + .collect(), + ); + let count: usize = statement.column_count(); + let mut cursor = statement.query(params_from_iter(params))?; + let mut rows: VecDeque = VecDeque::new(); + while let Some(row) = cursor.next()? { + rows.push_back(SqliteRow { + columns: Arc::clone(&columns), + values: (0..count) + .map(|index| row.get(index)) + .collect::>()?, + }); + } + Ok(SqliteCursor { + rows: Mutex::new(rows), + last_row_id: connection.last_insert_rowid(), + }) + }) + } +} + +#[pymethods] +impl SqliteConnection { + #[pyo3(signature = (sql, parameters = None))] + fn execute( + &self, + py: Python<'_>, + sql: &str, + parameters: Option<&Bound<'_, PyAny>>, + ) -> PyResult { + let mut params: Vec = Vec::new(); + if let Some(parameters) = parameters { + for value in parameters.try_iter()? { + let value = value?; + params.push(if value.is_none() { + Value::Null + } else if let Ok(value) = value.extract::() { + Value::Integer(value) + } else { + Value::Text(value.extract()?) + }); + } + } + self.execute_native(py, sql, ¶ms) + } + + fn executemany( + &self, + py: Python<'_>, + sql: &str, + parameters: &Bound<'_, PyAny>, + ) -> PyResult { + let mut result = SqliteCursor { + rows: Mutex::default(), + last_row_id: 0, + }; + for row in parameters.try_iter()? { + result = self.execute(py, sql, Some(&row?))?; + } + Ok(result) + } + + #[getter] + pub(crate) fn in_transaction(&self, py: Python<'_>) -> PyResult { + self.with_connection(py, |connection| Ok(!connection.is_autocommit())) + } + + pub(crate) fn close(&self, py: Python<'_>) -> PyResult<()> { + py.detach(|| { + self.connection + .lock() + .unwrap_or_else(PoisonError::into_inner) + .take(); + }); + Ok(()) + } +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/native_host/sqlite_cursor.rs b/crates/sqlbuild-python/src/bindings/_helpers/native_host/sqlite_cursor.rs new file mode 100644 index 0000000000..2a9796015c --- /dev/null +++ b/crates/sqlbuild-python/src/bindings/_helpers/native_host/sqlite_cursor.rs @@ -0,0 +1,99 @@ +//! Native SQLite rows, with a DB-API view for the existing connection override boundary. + +use std::collections::VecDeque; +use std::sync::{Arc, Mutex, PoisonError}; + +use pyo3::IntoPyObjectExt; +use pyo3::exceptions::{PyIndexError, PyTypeError}; +use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python, pyclass, pymethods}; +use pyo3::types::PyBytes; +use rusqlite::types::Value; + +#[pyclass(frozen, module = "sqlbuild._native", name = "_HistorySqliteRow")] +pub(crate) struct SqliteRow { + pub(crate) columns: Arc>, + pub(crate) values: Vec, +} + +impl SqliteRow { + pub(crate) fn value<'py>(&self, py: Python<'py>, index: usize) -> PyResult> { + match self.values.get(index) { + Some(Value::Null) => Ok(py.None().into_bound(py)), + Some(Value::Integer(value)) => value.into_bound_py_any(py), + Some(Value::Real(value)) => value.into_bound_py_any(py), + Some(Value::Text(value)) => value.into_bound_py_any(py), + Some(Value::Blob(value)) => Ok(PyBytes::new(py, value).into_any()), + None => Err(PyIndexError::new_err("tuple index out of range")), + } + } + + pub(crate) fn column<'py>(&self, py: Python<'py>, name: &str) -> PyResult> { + let index: usize = self + .columns + .iter() + .position(|column| column.eq_ignore_ascii_case(name)) + .ok_or_else(|| PyIndexError::new_err("No item with that key"))?; + self.value(py, index) + } +} + +#[pymethods] +impl SqliteRow { + fn __getitem__<'py>(&self, key: &Bound<'py, PyAny>) -> PyResult> { + if let Ok(index) = key.extract::() { + let index = if index < 0 { + self.values.len().checked_add_signed(index) + } else { + Some(index as usize) + }; + return self.value(key.py(), index.unwrap_or(usize::MAX)); + } + if let Ok(name) = key.extract::() { + return self.column(key.py(), &name); + } + Err(PyTypeError::new_err("Index must be int or string")) + } +} + +#[pyclass(frozen, module = "sqlbuild._native", name = "_HistorySqliteCursor")] +pub(crate) struct SqliteCursor { + pub(crate) rows: Mutex>, + pub(crate) last_row_id: i64, +} + +impl SqliteCursor { + pub(crate) fn next_row(&self) -> Option { + self.rows + .lock() + .unwrap_or_else(PoisonError::into_inner) + .pop_front() + } +} + +#[pymethods] +impl SqliteCursor { + fn __iter__(slf: Bound<'_, Self>) -> Bound<'_, Self> { + slf + } + + fn __next__(&self) -> Option { + self.next_row() + } + + fn fetchone(&self) -> Option { + self.next_row() + } + + fn fetchall(&self) -> Vec { + self.rows + .lock() + .unwrap_or_else(PoisonError::into_inner) + .drain(..) + .collect() + } + + #[getter] + fn lastrowid(&self) -> i64 { + self.last_row_id + } +} diff --git a/fensu.toml b/fensu.toml index d20d861f41..87d22cca82 100644 --- a/fensu.toml +++ b/fensu.toml @@ -1893,7 +1893,7 @@ pending_budgets = [ "crates/sqlbuild-python/src/bindings/_helpers/execution_history/conversion.rs=19", "crates/sqlbuild-python/src/bindings/_helpers/execution_history/functions.rs=2", "crates/sqlbuild-python/src/bindings/_helpers/execution_history/postgres.rs=28", - "crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs=33", + "crates/sqlbuild-python/src/bindings/_helpers/execution_history/sqlite.rs=28", "crates/sqlbuild-python/src/bindings/_helpers/execution_history/statements.rs=4", "crates/sqlbuild-python/src/bindings/_helpers/exported_constants/registration.rs=1", "crates/sqlbuild-python/src/bindings/_helpers/fingerprints/latest_rows.rs=16", diff --git a/src/sqlbuild/_native.pyi b/src/sqlbuild/_native.pyi index d50926603f..4a78cb2ea7 100644 --- a/src/sqlbuild/_native.pyi +++ b/src/sqlbuild/_native.pyi @@ -2978,8 +2978,9 @@ class _ExecutionHistoryStorage: ) -> bool | None: ... class SQLiteExecutionHistory(_ExecutionHistoryStorage): - """Project-local SQLite implementation of event-log and run-projection storage contracts.""" + """Project-local event-log and run-projection storage with a Rust-owned SQLite driver.""" + # Native connection by default; DB-API overrides retain their existing dispatch boundary. _connection: Any def __init__( self, diff --git a/tests/integration/src/sqlbuild/runtime/execution_history/test_sqlite_execution_history.py b/tests/integration/src/sqlbuild/runtime/execution_history/test_sqlite_execution_history.py index afc6d03d9d..19a74a589f 100644 --- a/tests/integration/src/sqlbuild/runtime/execution_history/test_sqlite_execution_history.py +++ b/tests/integration/src/sqlbuild/runtime/execution_history/test_sqlite_execution_history.py @@ -3,7 +3,6 @@ import sqlite3 import subprocess import sys -from collections.abc import Callable from concurrent.futures import Future, ThreadPoolExecutor, TimeoutError from datetime import UTC, datetime from pathlib import Path @@ -144,6 +143,31 @@ def test_given_project_directory_when_opening_sqlite_history_then_local_wal_is_h storage.close() +@pytest.mark.skipif(sys.platform == "win32", reason="Windows filenames cannot contain a colon") +@pytest.mark.parametrize( + "test_case", + ( + SQLitePathCase( + description="literal file scheme prefix", expected_filename="file:history.sqlite3" + ), + ), + ids=lambda case: case.description, +) +def test_given_uri_shaped_filename_when_opening_history_then_exact_literal_file_is_used( + tmp_path: Path, test_case: SQLitePathCase, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(tmp_path) + assert test_case.expected_filename is not None + path: Path = Path(test_case.expected_filename) + + with SQLiteExecutionHistory(path=path) as storage: + assert storage.check_health() is True + assert storage.path == path + + assert path.is_file() + assert not Path("history.sqlite3").exists() + + @pytest.mark.parametrize( "test_case", (SQLitePathCase(description="explicit memory database", expected_filename=None),), @@ -598,31 +622,13 @@ def test_given_post_connect_setup_failure_when_constructing_then_open_connection test_case: SQLitePersistenceCase, monkeypatch: pytest.MonkeyPatch, ) -> None: - opened: list[sqlite3.Connection] = [] - real_connect: Callable[..., sqlite3.Connection] = sqlite3.connect + opened: list[Any] = [] - def tracked_connect( - database: str, - *, - timeout: float, - isolation_level: None, - check_same_thread: bool, - ) -> sqlite3.Connection: - connection: sqlite3.Connection = real_connect( - database, - timeout=timeout, - isolation_level=isolation_level, - check_same_thread=check_same_thread, - ) - opened.append(connection) - return connection + def reject_migration(storage: SQLiteExecutionHistory) -> None: + opened.append(storage._connection) + raise OSError("controlled post-connect setup failure") - monkeypatch.setattr(sqlite3, "connect", tracked_connect) - monkeypatch.setattr( - SQLiteExecutionHistory, - "_migrate", - Mock(side_effect=OSError("controlled post-connect setup failure")), - ) + monkeypatch.setattr(SQLiteExecutionHistory, "_migrate", reject_migration) with pytest.raises(ExecutionHistoryStorageError, match="cannot open SQLite history"): SQLiteExecutionHistory(path=tmp_path / "history.sqlite3") @@ -633,6 +639,42 @@ def tracked_connect( assert test_case.expected_run_count == 0 +@pytest.mark.parametrize( + "test_case", + ( + SQLitePersistenceCase( + description="native driver persists and reopens without Python connect", + expected_event_count=1, + expected_run_count=1, + ), + ), + ids=lambda case: case.description, +) +def test_given_python_driver_unavailable_when_persisting_then_native_history_reopens( + tmp_path: Path, + test_case: SQLitePersistenceCase, + monkeypatch: pytest.MonkeyPatch, +) -> None: + path: Path = tmp_path / "history.sqlite3" + event: LifecycleEvent = lifecycle_event("native-driver") + monkeypatch.setattr( + sqlite3, "connect", Mock(side_effect=RuntimeError("Python driver unavailable")) + ) + + with SQLiteExecutionHistory(path=path) as storage: + stored: StoredEvent = storage.append_event(event) + storage.project((stored,)) + + with SQLiteExecutionHistory(path=path) as reopened: + page: EventPage = reopened.get_events(event_filter=EventFilter()) + runs: RunPage = reopened.get_runs(run_filter=RunFilter()) + assert reopened.check_health() is True + + assert len(page.records) == test_case.expected_event_count + assert page.records[0].event == stored.event + assert len(runs.records) == test_case.expected_run_count + + @pytest.mark.skipif(sys.platform == "win32", reason="subprocess SIGINT delivery requires POSIX") @pytest.mark.parametrize( "test_case", diff --git a/tests/unit/src/sqlbuild/runtime/execution_history/conformance/conftest.py b/tests/unit/src/sqlbuild/runtime/execution_history/conformance/conftest.py index 42a8d417e3..1a104c79af 100644 --- a/tests/unit/src/sqlbuild/runtime/execution_history/conformance/conftest.py +++ b/tests/unit/src/sqlbuild/runtime/execution_history/conformance/conftest.py @@ -580,7 +580,7 @@ def sqlite_atomic_failing_factory() -> RunStorage: expected_backend="memory", ), BackendCase( - description="stdlib SQLite backend", + description="native SQLite backend", event_log_factory=sqlite_factory, run_storage_factory=sqlite_factory, append_failing_event_log_factory=sqlite_append_failing_factory,