diff --git a/core/incremental/expr_compiler.rs b/core/incremental/expr_compiler.rs index 310922633..9ea22028a 100644 --- a/core/incremental/expr_compiler.rs +++ b/core/incremental/expr_compiler.rs @@ -189,17 +189,25 @@ impl std::fmt::Debug for CompiledExpression { } } +#[derive(PartialEq)] +enum TrivialType { + Integer, + Float, + Text, + Null, +} + impl CompiledExpression { /// Get the "type" of a trivial expression for type checking /// Returns None if type can't be determined statically - fn get_trivial_type(expr: &TrivialExpression) -> Option<&'static str> { + fn get_trivial_type(expr: &TrivialExpression) -> Option { match expr { TrivialExpression::Column(_) => None, // Can't know column type statically TrivialExpression::Immediate(val) => match val { - Value::Integer(_) => Some("integer"), - Value::Float(_) => Some("float"), - Value::Text(_) => Some("text"), - Value::Null => Some("null"), + Value::Integer(_) => Some(TrivialType::Integer), + Value::Float(_) => Some(TrivialType::Float), + Value::Text(_) => Some(TrivialType::Text), + Value::Null => Some(TrivialType::Null), _ => None, }, TrivialExpression::Binary { left, right, .. } => { @@ -215,9 +223,12 @@ impl CompiledExpression { } } - /// Check if an expression is trivial (columns, immediates, and simple arithmetic) - /// Only considers expressions trivial if they don't require type coercion - fn is_trivial_expr(expr: &Expr, input_column_names: &[String]) -> Option { + // Validates if an expression is trivial (columns, immediates, and simple arithmetic) + // Only considers expressions trivial if they don't require type coercion + fn try_get_trivial_expr( + expr: &Expr, + input_column_names: &[String], + ) -> Option { match expr { // Column reference or register Expr::Id(name) => input_column_names @@ -254,8 +265,8 @@ impl CompiledExpression { match op { Operator::Add | Operator::Subtract | Operator::Multiply | Operator::Divide => { // Both operands must be trivial - let left_trivial = Self::is_trivial_expr(left, input_column_names)?; - let right_trivial = Self::is_trivial_expr(right, input_column_names)?; + let left_trivial = Self::try_get_trivial_expr(left, input_column_names)?; + let right_trivial = Self::try_get_trivial_expr(right, input_column_names)?; // Check if we can determine types statically // If both are immediates, they must have the same type @@ -267,8 +278,8 @@ impl CompiledExpression { ) { // Both types are known - they must match (or one is null) if left_type != right_type - && left_type != "null" - && right_type != "null" + && left_type != TrivialType::Null + && right_type != TrivialType::Null { return None; // Type mismatch - not trivial } @@ -288,7 +299,7 @@ impl CompiledExpression { // Parenthesized expressions with single element Expr::Parenthesized(exprs) if exprs.len() == 1 => { - Self::is_trivial_expr(&exprs[0], input_column_names) + Self::try_get_trivial_expr(&exprs[0], input_column_names) } _ => None, @@ -309,7 +320,7 @@ impl CompiledExpression { let input_count = input_column_names.len(); // First, check if this is a trivial expression - if let Some(trivial) = Self::is_trivial_expr(expr, input_column_names) { + if let Some(trivial) = Self::try_get_trivial_expr(expr, input_column_names) { return Ok(CompiledExpression { executor: ExpressionExecutor::Trivial(trivial), input_count, @@ -389,7 +400,14 @@ impl CompiledExpression { let mut state = ProgramState::new(program.max_registers, 0); // Load input values into registers - for (idx, value) in values.iter().take(self.input_count).enumerate() { + assert_eq!( + values.len(), + self.input_count, + "Mismatch in number of registers! Got {}, expected {}", + values.len(), + self.input_count + ); + for (idx, value) in values.iter().enumerate() { state.set_register(idx, Register::Value(value.clone())); } diff --git a/core/incremental/operator.rs b/core/incremental/operator.rs index 9b0a9d15a..34f9b3ff0 100644 --- a/core/incremental/operator.rs +++ b/core/incremental/operator.rs @@ -651,111 +651,71 @@ impl ProjectOperator { None }; // Try to compile the expression (handles both columns and complex expressions) - match CompiledExpression::compile( + let compiled = CompiledExpression::compile( expr, &input_column_names, schema, &temp_syms, internal_conn.clone(), - ) { - Ok(compiled) => { - columns.push(ProjectColumn { - expr: expr.clone(), - alias: alias_str, - compiled, - }); - } - Err(_) => { - // If compilation fails, skip this column for now - // In the future we might want to handle this better - } - } + )?; + columns.push(ProjectColumn { + expr: expr.clone(), + alias: alias_str, + compiled, + }); } ResultColumn::Star => { // Select all columns - create trivial column references for name in &input_column_names { // Create an Id expression for the column let expr = Expr::Id(Name::Ident(name.clone())); - // This should always compile successfully as a trivial column reference - if let Ok(compiled) = CompiledExpression::compile( + let compiled = CompiledExpression::compile( &expr, &input_column_names, schema, &temp_syms, internal_conn.clone(), - ) { - columns.push(ProjectColumn { - expr, - alias: None, - compiled, - }); - } + )?; + columns.push(ProjectColumn { + expr, + alias: None, + compiled, + }); } } - _ => { - // For now, skip TableStar and other cases + x => { + return Err(crate::LimboError::ParseError(format!( + "Unsupported {x:?} clause when compiling project operator", + ))); } } } if columns.is_empty() { - // If no columns were extracted, default to projecting all input columns - input_column_names - .iter() - .filter_map(|name| { - let expr = Expr::Id(Name::Ident(name.clone())); - CompiledExpression::compile( - &expr, - &input_column_names, - schema, - &temp_syms, - internal_conn.clone(), - ) - .ok() - .map(|compiled| ProjectColumn { - expr, - alias: None, - compiled, - }) - }) - .collect() - } else { - columns + return Err(crate::LimboError::ParseError( + "No columns found when compiling project operator".to_string(), + )); } + columns } else { - // Not a simple SELECT statement, default to projecting all columns - input_column_names - .iter() - .filter_map(|name| { - let expr = Expr::Id(Name::Ident(name.clone())); - CompiledExpression::compile( - &expr, - &input_column_names, - schema, - &temp_syms, - internal_conn.clone(), - ) - .ok() - .map(|compiled| ProjectColumn { - expr, - alias: None, - compiled, - }) - }) - .collect() + return Err(crate::LimboError::ParseError( + "Expression is not a valid SELECT expression".to_string(), + )); }; // Generate output column names based on aliases or expressions let output_column_names = columns .iter() .map(|c| { - c.alias.clone().unwrap_or_else(|| { - // For simple column references, use the column name - if let Expr::Id(name) = &c.expr { - name.as_str().to_string() - } else { - "expr".to_string() + c.alias.clone().unwrap_or_else(|| match &c.expr { + Expr::Id(name) => name.as_str().to_string(), + Expr::Qualified(table, column) => { + format!("{}.{}", table.as_str(), column.as_str()) } + Expr::DoublyQualified(db, table, column) => { + format!("{}.{}.{}", db.as_str(), table.as_str(), column.as_str()) + } + _ => c.expr.to_string(), }) }) .collect(); @@ -786,11 +746,7 @@ impl ProjectOperator { let result = col .compiled .execute(values, internal_pager) - .unwrap_or_else(|_| { - // Fall back to manual evaluation on error - // This can happen for expressions with unsupported operations - self.evaluate_expression(&col.expr, values) - }); + .expect("Failed to execute compiled expression for the Project operator"); output.push(result); }