Skip to content

Commit b6805de

Browse files
teunbrandclaude
andcommitted
Execute side-effect statements before trailing SELECT
transform_global_sql returned empty side-effects when a SELECT was present, so CREATE/INSERT preambles were skipped. Hoist side-effect extraction so it runs regardless of the query type. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent db681e9 commit b6805de

2 files changed

Lines changed: 46 additions & 24 deletions

File tree

src/execute/cte.rs

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -219,18 +219,39 @@ pub fn transform_global_sql(
219219
source_tree: &SourceTree,
220220
materialized_ctes: &HashSet<String>,
221221
) -> (Vec<String>, Option<String>) {
222+
// Collect side-effect statements (CREATE, INSERT, UPDATE, DELETE) that
223+
// need to run before the main query. These appear alongside a trailing
224+
// SELECT or VISUALISE FROM.
225+
//
226+
// Only structured DML is handled here — other_sql_statement nodes
227+
// (INSTALL, LOAD, SET, …) are pre-executed in prepare_data_with_reader.
228+
let root = source_tree.root();
229+
230+
let side_effect_stmts = r#"
231+
(sql_statement
232+
[(create_statement)
233+
(insert_statement)
234+
(update_statement)
235+
(delete_statement)] @stmt)
236+
"#;
237+
let side_effects: Vec<String> = source_tree
238+
.find_texts(&root, side_effect_stmts)
239+
.into_iter()
240+
.map(|s| transform_cte_references(s.trim(), materialized_ctes))
241+
.filter(|s| !s.is_empty())
242+
.collect();
243+
222244
// Try to extract trailing SELECT (WITH...SELECT or direct SELECT)
223245
let select_sql = split_with_query(source_tree)
224246
.map(|(_, select)| select)
225247
.or_else(|| {
226248
// Fallback: direct SELECT statement (no WITH clause)
227-
let root = source_tree.root();
228249
source_tree.find_text(&root, "(sql_statement (select_statement) @select)")
229250
});
230251

231252
if let Some(select_sql) = select_sql {
232253
return (
233-
vec![],
254+
side_effects,
234255
Some(transform_cte_references(&select_sql, materialized_ctes)),
235256
);
236257
}
@@ -239,30 +260,10 @@ pub fn transform_global_sql(
239260
return (vec![], None);
240261
}
241262

242-
// We have non-SELECT executable SQL (CREATE, INSERT, …) and/or
243-
// VISUALISE FROM. Split them: side-effect statements run directly,
244-
// VISUALISE FROM becomes the queryable part.
245-
//
246-
// Only structured DML is handled here — other_sql_statement nodes
247-
// (INSTALL, LOAD, SET, …) are pre-executed in prepare_data_with_reader.
263+
// We have non-SELECT executable SQL and/or VISUALISE FROM.
264+
// Side-effects run directly, VISUALISE FROM becomes the queryable part.
248265
// A bare WITH clause without a trailing statement is not executable on
249266
// its own (its CTEs are already materialized separately).
250-
let root = source_tree.root();
251-
252-
let side_effect_stmts = r#"
253-
(sql_statement
254-
[(create_statement)
255-
(insert_statement)
256-
(update_statement)
257-
(delete_statement)] @stmt)
258-
"#;
259-
let side_effects: Vec<String> = source_tree
260-
.find_texts(&root, side_effect_stmts)
261-
.into_iter()
262-
.map(|s| transform_cte_references(s.trim(), materialized_ctes))
263-
.filter(|s| !s.is_empty())
264-
.collect();
265-
266267
let viz_from_query = source_tree
267268
.find_text(
268269
&root,

src/execute/mod.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1947,6 +1947,27 @@ mod tests {
19471947
assert_eq!(result.data.get(key).unwrap().height(), 3);
19481948
}
19491949

1950+
#[cfg(feature = "duckdb")]
1951+
#[test]
1952+
fn test_select_after_create_and_insert() {
1953+
let reader = DuckDBReader::from_connection_string("duckdb://memory").unwrap();
1954+
1955+
let query = r#"
1956+
CREATE TEMP TABLE data(x INTEGER, y INTEGER);
1957+
INSERT INTO data VALUES (1, 10), (2, 20), (3, 30);
1958+
SELECT * FROM data
1959+
VISUALISE x, y
1960+
DRAW point
1961+
"#;
1962+
1963+
let result = prepare_data_with_reader(query, &reader).unwrap();
1964+
let key = result.specs[0].layers[0]
1965+
.data_key
1966+
.as_ref()
1967+
.expect("Layer should have data_key");
1968+
assert_eq!(result.data.get(key).unwrap().height(), 3);
1969+
}
1970+
19501971
/// Test that literal mappings survive stat transforms (e.g., histogram grouping).
19511972
///
19521973
/// This tests the fix for issue #129 where literal aesthetic columns like

0 commit comments

Comments
 (0)