Merge remote-tracking branch 'upstream/main' into pr2-hybrid-reader

# Conflicts:
#	CHANGELOG.md

Author: jimhester

CHANGELOG.md

@@ -4,6 +4,14 @@
 
 - New `HybridReader` that composes any primary `Reader` with an in-process `DuckDBReader` for staging. `register()` writes to staging; `execute_sql` routes queries that reference registered names to staging and everything else to the primary. Available behind the existing `duckdb` feature.
 
+### Fixed
+
+- Side effects like `CREATE TEMP TABLE` before the `VISUALISE` statement are now
+  separated from directly feeding into the visualisation data (#415)
+- Fixed bug where panel axes were unintentionally anchored to zero when using 
+  `FACET ... SETTING free => 'x'/'y'` (#410).
+- Fixed bug where faceted data were matched to the incorrect panels (#409)
+
 ## 0.3.1 - 2026-04-30
 
 ### Fixed

src/execute/cte.rs

@@ -210,35 +210,79 @@ pub fn split_with_query(source_tree: &SourceTree) -> Option<(String, String)> {
     Some((cte_prefix, trailing))
 }
 
-/// Transform global SQL for execution with temp tables
+/// Transform global SQL for execution with temp tables.
 ///
-/// If the SQL has a WITH clause followed by SELECT, extracts just the SELECT
-/// portion and transforms CTE references to temp table names.
-/// For SQL without WITH clause, just transforms any CTE references.
+/// Returns statements to execute directly as side effects (CREATE, INSERT, …)
+/// and an optional query whose result should be wrapped as the global temp
+/// table.
 pub fn transform_global_sql(
     source_tree: &SourceTree,
     materialized_ctes: &HashSet<String>,
-) -> Option<String> {
+) -> (Vec<String>, Option<String>) {
+    // Collect side-effect statements (CREATE, INSERT, UPDATE, DELETE) that
+    // need to run before the main query. These appear alongside a trailing
+    // SELECT or VISUALISE FROM.
+    //
+    // Only structured DML is handled here — other_sql_statement nodes
+    // (INSTALL, LOAD, SET, …) are pre-executed in prepare_data_with_reader.
+    let root = source_tree.root();
+
+    let side_effect_stmts = r#"
+        (sql_statement
+          [(create_statement)
+           (insert_statement)
+           (update_statement)
+           (delete_statement)] @stmt)
+    "#;
+    let side_effects: Vec<String> = source_tree
+        .find_texts(&root, side_effect_stmts)
+        .into_iter()
+        .map(|s| transform_cte_references(s.trim(), materialized_ctes))
+        .filter(|s| !s.is_empty())
+        .collect();
+
     // Try to extract trailing SELECT (WITH...SELECT or direct SELECT)
     let select_sql = split_with_query(source_tree)
         .map(|(_, select)| select)
         .or_else(|| {
             // Fallback: direct SELECT statement (no WITH clause)
-            let root = source_tree.root();
             source_tree.find_text(&root, "(sql_statement (select_statement) @select)")
         });
 
     if let Some(select_sql) = select_sql {
-        Some(transform_cte_references(&select_sql, materialized_ctes))
-    } else if does_consume_cte(source_tree) {
-        // Non-SELECT executable SQL (CREATE, INSERT, UPDATE, DELETE)
-        // OR VISUALISE FROM (which injects SELECT * FROM <source>)
-        // Extract SQL (with injection if VISUALISE FROM) and transform CTE references
-        let sql = source_tree.extract_sql()?;
-        Some(transform_cte_references(&sql, materialized_ctes))
+        return (
+            side_effects,
+            Some(transform_cte_references(&select_sql, materialized_ctes)),
+        );
+    }
+
+    if !has_executable_sql(source_tree) {
+        return (vec![], None);
+    }
+
+    // We have non-SELECT executable SQL and/or VISUALISE FROM.
+    // Side-effects run directly, VISUALISE FROM becomes the queryable part.
+    // A bare WITH clause without a trailing statement is not executable on
+    // its own (its CTEs are already materialized separately).
+    let viz_from_query = source_tree
+        .find_text(
+            &root,
+            r#"(visualise_statement (from_clause (table_ref) @table))"#,
+        )
+        .map(|table| {
+            let q = format!("SELECT * FROM {}", table);
+            transform_cte_references(&q, materialized_ctes)
+        });
+
+    if !side_effects.is_empty() || viz_from_query.is_some() {
+        (side_effects, viz_from_query)
     } else {
-        // No executable SQL (just CTEs)
-        None
+        // has_executable_sql was true but we found no specific statements or
+        // VISUALISE FROM — fall back to extract_sql as the query.
+        let query = source_tree
+            .extract_sql()
+            .map(|s| transform_cte_references(&s, materialized_ctes));
+        (vec![], query)
     }
 }
 
@@ -248,7 +292,7 @@ pub fn transform_global_sql(
 /// This handles cases like `WITH a AS (...), b AS (...) VISUALISE` where the WITH
 /// clause has no trailing SELECT - these CTEs are still extracted for layer use
 /// but shouldn't be executed as global data.
-pub fn does_consume_cte(source_tree: &SourceTree) -> bool {
+pub fn has_executable_sql(source_tree: &SourceTree) -> bool {
     let root = source_tree.root();
 
     // Check for direct executable statements (SELECT, CREATE, INSERT, UPDATE,

src/execute/mod.rs

@@ -1043,7 +1043,9 @@ pub fn prepare_data_with_reader(query: &str, reader: &dyn Reader) -> Result<Prep
         ));
     }
 
-    // Execute setup statements (INSTALL, LOAD, SET, etc.) before the main query
+    // Execute setup statements (INSTALL, LOAD, SET, etc.) before the main query.
+    // Structured DML (CREATE, INSERT, UPDATE, DELETE) is handled separately as
+    // side-effects in cte::transform_global_sql.
     for stmt in source_tree.find_texts(&root, "(sql_statement (other_sql_statement) @stmt)") {
         execute_query(&stmt)?;
     }
@@ -1063,16 +1065,21 @@ pub fn prepare_data_with_reader(query: &str, reader: &dyn Reader) -> Result<Prep
     // Execute global SQL if present
     // If there's a WITH clause, extract just the trailing SELECT and transform CTE references.
     // The global result is stored as a temp table so filtered layers can query it efficiently.
-    // Track whether we actually create the temp table (depends on transform_global_sql succeeding)
     let mut has_global_table = false;
     if sql_part.is_some() {
-        if let Some(transformed_sql) = cte::transform_global_sql(&source_tree, &materialized_ctes) {
+        let (side_effects, query) = cte::transform_global_sql(&source_tree, &materialized_ctes);
+
+        for stmt in &side_effects {
+            execute_query(stmt)?;
+        }
+
+        if let Some(query) = query {
             // Materialize global result as a temp table directly on the backend
             // (no roundtrip through Rust).
             let statements = reader.dialect().create_or_replace_temp_table_sql(
                 &naming::global_table(),
                 &[],
-                &transformed_sql,
+                &query,
             );
             for stmt in &statements {
                 execute_query(stmt)?;
@@ -1895,6 +1902,72 @@ mod tests {
         assert_eq!(result.data.get(layer1_key).unwrap().height(), 3);
     }
 
+    #[cfg(feature = "duckdb")]
+    #[test]
+    fn test_visualise_from_after_create() {
+        let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap();
+
+        let query = r#"
+            CREATE TEMP TABLE data(x, y) AS (VALUES
+              ('A', 5),
+              ('B', 2),
+              ('C', 4),
+              ('D', 7),
+              ('E', 6)
+            )
+            VISUALISE x, y FROM data
+            DRAW area
+        "#;
+
+        let result = prepare_data_with_reader(query, &reader).unwrap();
+        let key = result.specs[0].layers[0]
+            .data_key
+            .as_ref()
+            .expect("Layer should have data_key");
+        assert_eq!(result.data.get(key).unwrap().height(), 5);
+    }
+
+    #[cfg(feature = "duckdb")]
+    #[test]
+    fn test_visualise_from_after_create_and_insert() {
+        let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap();
+
+        let query = r#"
+            CREATE TEMP TABLE data(x INTEGER, y INTEGER);
+            INSERT INTO data VALUES (1, 10), (2, 20), (3, 30);
+            VISUALISE x, y FROM data
+            DRAW point
+        "#;
+
+        let result = prepare_data_with_reader(query, &reader).unwrap();
+        let key = result.specs[0].layers[0]
+            .data_key
+            .as_ref()
+            .expect("Layer should have data_key");
+        assert_eq!(result.data.get(key).unwrap().height(), 3);
+    }
+
+    #[cfg(feature = "duckdb")]
+    #[test]
+    fn test_select_after_create_and_insert() {
+        let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap();
+
+        let query = r#"
+            CREATE TEMP TABLE data(x INTEGER, y INTEGER);
+            INSERT INTO data VALUES (1, 10), (2, 20), (3, 30);
+            SELECT * FROM data
+            VISUALISE x, y
+            DRAW point
+        "#;
+
+        let result = prepare_data_with_reader(query, &reader).unwrap();
+        let key = result.specs[0].layers[0]
+            .data_key
+            .as_ref()
+            .expect("Layer should have data_key");
+        assert_eq!(result.data.get(key).unwrap().height(), 3);
+    }
+
     /// Test that literal mappings survive stat transforms (e.g., histogram grouping).
     ///
     /// This tests the fix for issue #129 where literal aesthetic columns like

src/writer/vegalite/encoding.rs

@@ -924,11 +924,8 @@ fn build_column_encoding(
 
     // Position scales don't include zero by default — but only when we set
     // an explicit domain. With free facet scales (no domain), VL computes
-    // the domain from data values. Setting zero:false in that case can exclude
-    // 0 from the domain, breaking charts with pre-computed stacking (y2/theta2
-    // starts at 0). Let VL's defaults handle it instead.
-    let is_free = is_position_free_for_aesthetic(aesthetic, ctx.free_scales);
-    if aesthetic_ctx.is_primary_internal(aesthetic) && !is_free {
+    // the domain from data values.
+    if aesthetic_ctx.is_primary_internal(aesthetic) {
         scale_obj.insert("zero".to_string(), json!(false));
     }