Skip to content

Commit a2a598b

Browse files
TimelordUKclaude
andcommitted
fix: Set from_source in ExpressionLifter to fix QUALIFY regression
Fixes the QUALIFY clause regression introduced by the from_source migration. The issue was that ExpressionLifter was only setting the deprecated from_table field when creating CTEs, not the new from_source field. This caused the query engine to not properly recognize CTE references. Changes: - Updated ExpressionLifter to set both from_source (preferred) and from_table (deprecated) - Added logic in StatementExecutor to re-extract base table when preprocessing adds CTEs - All 28 formal tests now passing (including qualify.sql) - All 396 Rust unit tests passing The ExpressionLifter creates CTEs like: WITH __lifted_1 AS (SELECT *, ROW_NUMBER() ... AS rn FROM sales) SELECT ... FROM __lifted_1 Now from_source is properly set to TableSource::Table("__lifted_1") so the query engine can find the CTE in the cte_context and use it as the source table. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent bf857c4 commit a2a598b

2 files changed

Lines changed: 24 additions & 3 deletions

File tree

src/execution/statement_executor.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,22 @@ impl StatementExecutor {
154154
stats.preprocessing_time_ms = preprocess_start.elapsed().as_secs_f64() * 1000.0;
155155
stats.preprocessing_applied = preprocessing_applied;
156156

157+
// Step 2.5: Re-check source table if preprocessing added CTEs
158+
// If the transformed statement has CTEs (e.g., from ExpressionLifter),
159+
// we need to use the source table that was passed in, not the extracted base table,
160+
// because the CTE itself becomes the data source.
161+
let final_source_table = if !transformed_stmt.ctes.is_empty() {
162+
// Has CTEs - need to extract base table from the actual CTE definitions
163+
// to get the underlying data source
164+
Self::extract_base_table(&transformed_stmt, context)
165+
} else {
166+
source_table
167+
};
168+
157169
// Step 3: Execute the transformed statement directly via QueryEngine
158170
let exec_start = Instant::now();
159-
let result_view = self.execute_ast(transformed_stmt.clone(), source_table, context)?;
171+
let result_view =
172+
self.execute_ast(transformed_stmt.clone(), final_source_table, context)?;
160173
stats.execution_time_ms = exec_start.elapsed().as_secs_f64() * 1000.0;
161174

162175
// Step 4: If this was a SELECT INTO statement, store the result as a temp table

src/query_plan/expression_lifter.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,16 @@ impl ExpressionLifter {
354354
}
355355

356356
stmt.select_items = new_select_items;
357-
stmt.from_table = Some(cte_name.clone());
358-
stmt.from_subquery = None;
357+
// Set from_source to reference the CTE (preferred)
358+
stmt.from_source = Some(crate::sql::parser::ast::TableSource::Table(
359+
cte_name.clone(),
360+
));
361+
// Also set deprecated field for backward compatibility
362+
#[allow(deprecated)]
363+
{
364+
stmt.from_table = Some(cte_name.clone());
365+
stmt.from_subquery = None;
366+
}
359367
stmt.where_clause = None; // Already in the CTE
360368

361369
CTE {

0 commit comments

Comments
 (0)