From a87ad434067e2240ee35f8644422cd508b0c954d Mon Sep 17 00:00:00 2001 From: YoEight Date: Sun, 4 Jan 2026 14:14:57 -0500 Subject: [PATCH 01/11] start working on aggregate function checks --- src/analysis.rs | 43 ++++++++++++++++++++++++++++++++++++++++++- src/ast.rs | 8 ++++++-- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/analysis.rs b/src/analysis.rs index 5e639d0..2787856 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -113,6 +113,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -120,6 +121,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -127,6 +129,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -134,6 +137,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -141,6 +145,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -148,6 +153,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -155,6 +161,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number, Type::Number], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -162,6 +169,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -169,6 +177,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -176,6 +185,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -183,6 +193,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::String), + aggregate: false, }, ), ( @@ -190,6 +201,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::String), + aggregate: false, }, ), ( @@ -197,6 +209,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::String), + aggregate: false, }, ), ( @@ -204,6 +217,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::String), + aggregate: false, }, ), ( @@ -211,6 +225,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::String), + aggregate: false, }, ), ( @@ -218,6 +233,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -225,6 +241,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -232,6 +249,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String, Type::Number, Type::Number], result: Box::new(Type::String), + aggregate: false, }, ), ( @@ -239,6 +257,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String, Type::String, Type::String], result: Box::new(Type::String), + aggregate: false, }, ), ( @@ -246,6 +265,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String, Type::String], result: Box::new(Type::Bool), + aggregate: false, }, ), ( @@ -253,6 +273,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String, Type::String], result: Box::new(Type::Bool), + aggregate: false, }, ), ( @@ -260,6 +281,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![], result: Box::new(Type::String), + aggregate: false, }, ), ( @@ -267,6 +289,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -274,6 +297,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -281,6 +305,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -288,6 +313,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -295,6 +321,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -302,6 +329,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -309,6 +337,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::String], result: Box::new(Type::Number), + aggregate: false, }, ), ( @@ -316,6 +345,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Bool, Type::Unspecified, Type::Unspecified], result: Box::new(Type::Unspecified), + aggregate: false, }, ), ( @@ -323,6 +353,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![], result: Box::new(Type::Number), + aggregate: true, }, ), ( @@ -330,6 +361,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: true, }, ), ( @@ -337,6 +369,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: true, }, ), ( @@ -344,6 +377,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: true, }, ), ( @@ -351,6 +385,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: true, }, ), ( @@ -358,6 +393,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: true, }, ), ( @@ -365,6 +401,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: true, }, ), ( @@ -372,6 +409,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: true, }, ), ( @@ -379,6 +417,7 @@ impl Default for AnalysisOptions { Type::App { args: vec![Type::Number], result: Box::new(Type::Number), + aggregate: true, }, ), ]), @@ -695,12 +734,13 @@ impl<'a> Analysis<'a> { this @ Value::Access(_) => Ok(self.analyze_access(attrs, this, expect)?), this @ Value::App(app) => { + // TODO - we should still check if the function exists if matches!(expect, Type::Unspecified) { return Ok(self.project_type(this)); } match expect { - Type::App { args, mut result } if app.args.len() == args.len() => { + Type::App { args, mut result, aggregate } if app.args.len() == args.len() => { let mut arg_types = Vec::with_capacity(args.capacity()); for (arg, tpe) in app.args.iter().zip(args.into_iter()) { arg_types.push(self.analyze_expr(arg, tpe)?); @@ -714,6 +754,7 @@ impl<'a> Analysis<'a> { Ok(Type::App { args: arg_types, result, + aggregate }) } else { Err(AnalysisError::FuncUndeclared( diff --git a/src/ast.rs b/src/ast.rs index 0bc56ae..890e7e9 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -72,7 +72,7 @@ pub enum Type { /// Subject pattern type Subject, /// Function type - App { args: Vec, result: Box }, + App { args: Vec, result: Box, aggregate: bool }, /// Date type (e.g., `2026-01-03`) /// /// Used when a field is explicitly converted to a date using the `AS DATE` syntax. @@ -169,18 +169,21 @@ impl Type { Self::App { args: mut a_args, result: mut a_res, + aggregate: a_agg, }, Self::App { args: b_args, result: b_res, + aggregate: b_agg, }, - ) if a_args.len() == b_args.len() => { + ) if a_args.len() == b_args.len() && a_agg == b_agg => { if a_args.is_empty() { let tmp = mem::take(a_res.as_mut()); *a_res = tmp.check(attrs, *b_res)?; return Ok(Self::App { args: a_args, result: a_res, + aggregate: a_agg, }); } @@ -195,6 +198,7 @@ impl Type { Ok(Self::App { args: a_args, result: a_res, + aggregate: a_agg, }) } From c7661ad433429eb338d7455bc720ee8bcbf20581 Mon Sep 17 00:00:00 2001 From: YoEight Date: Sun, 4 Jan 2026 14:54:15 -0500 Subject: [PATCH 02/11] improve function application type checking --- src/analysis.rs | 58 ++++++++++++++++++++++--------------------------- src/ast.rs | 6 ++++- src/error.rs | 3 +++ 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/src/analysis.rs b/src/analysis.rs index 2787856..cea7080 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -733,44 +733,38 @@ impl<'a> Analysis<'a> { this @ Value::Access(_) => Ok(self.analyze_access(attrs, this, expect)?), - this @ Value::App(app) => { - // TODO - we should still check if the function exists - if matches!(expect, Type::Unspecified) { - return Ok(self.project_type(this)); - } + Value::App(app) => { + if let Some(tpe) = self.options.default_scope.entries.get(app.func.as_str()) + && let Type::App { args, result, aggregate } = tpe + { + if args.len() != app.args.len() { + return Err(AnalysisError::FunWrongArgumentCount( + attrs.pos.line, + attrs.pos.col, + app.func.clone(), + args.len(), + app.args.len(), + )); + } - match expect { - Type::App { args, mut result, aggregate } if app.args.len() == args.len() => { - let mut arg_types = Vec::with_capacity(args.capacity()); - for (arg, tpe) in app.args.iter().zip(args.into_iter()) { - arg_types.push(self.analyze_expr(arg, tpe)?); - } + for (arg, tpe) in app.args.iter().zip(args.iter().cloned()) { + self.analyze_expr(arg, tpe)?; + } - if let Some(tpe) = self.options.default_scope.entries.get(app.func.as_str()) - { - let tmp = mem::take(result.as_mut()); - *result = tmp.check(attrs, tpe.clone())?; + // TODO - check if we are dealing with an aggregate function while not in a + // projection expression. - Ok(Type::App { - args: arg_types, - result, - aggregate - }) - } else { - Err(AnalysisError::FuncUndeclared( - attrs.pos.line, - attrs.pos.col, - app.func.clone(), - )) - } + if matches!(expect, Type::Unspecified) { + Ok(result.as_ref().clone()) + } else { + expect.check(attrs, result.as_ref().clone()) } - - expect => Err(AnalysisError::TypeMismatch( + } else { + Err(AnalysisError::FuncUndeclared( attrs.pos.line, attrs.pos.col, - expect, - self.project_type(value), - )), + app.func.clone(), + )) } } diff --git a/src/ast.rs b/src/ast.rs index 890e7e9..14e6ff1 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -72,7 +72,11 @@ pub enum Type { /// Subject pattern type Subject, /// Function type - App { args: Vec, result: Box, aggregate: bool }, + App { + args: Vec, + result: Box, + aggregate: bool, + }, /// Date type (e.g., `2026-01-03`) /// /// Used when a field is explicitly converted to a date using the `AS DATE` syntax. diff --git a/src/error.rs b/src/error.rs index a27719d..c8aec9f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -192,6 +192,9 @@ pub enum AnalysisError { /// not registered in the `AnalysisOptions` custom type set. #[error("{0}:{1}: unsupported custom type '{2}'")] UnsupportedCustomType(u32, u32, String), + + #[error("{0}:{1}: function '{2}' requires {3} parameters but got {4}")] + FunWrongArgumentCount(u32, u32, String, usize, usize), } impl From for Error { From dcef92b803f851e0a5abebae4ab4d013014435af Mon Sep 17 00:00:00 2001 From: YoEight Date: Sun, 4 Jan 2026 16:14:57 -0500 Subject: [PATCH 03/11] feat: disallow using source-based fields with aggregate functions --- src/analysis.rs | 105 ++++++++++++++++++++++++++++++++++++------------ src/error.rs | 6 +++ 2 files changed, 85 insertions(+), 26 deletions(-) diff --git a/src/analysis.rs b/src/analysis.rs index cea7080..d26fa00 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -486,6 +486,13 @@ impl Scope { } } +#[derive(Default)] +struct Context { + analyzing_projection: bool, + use_agg_func: bool, + use_only_constant_expr: bool, +} + struct Analysis<'a> { options: &'a AnalysisOptions, prev_scopes: Vec, @@ -528,7 +535,7 @@ impl<'a> Analysis<'a> { } if let Some(expr) = &query.predicate { - self.analyze_expr(expr, Type::Bool)?; + self.analyze_expr(&mut Context::default(), expr, Type::Bool)?; } if let Some(group_by) = &query.group_by { @@ -539,10 +546,10 @@ impl<'a> Analysis<'a> { )); } - self.analyze_expr(&group_by.expr, Type::Unspecified)?; + self.analyze_expr(&mut Context::default(), &group_by.expr, Type::Unspecified)?; if let Some(expr) = &group_by.predicate { - self.analyze_expr(expr, Type::Bool)?; + self.analyze_expr(&mut Context::default(), expr, Type::Bool)?; } } @@ -553,7 +560,7 @@ impl<'a> Analysis<'a> { order_by.expr.attrs.pos.col, )); } - self.analyze_expr(&order_by.expr, Type::Unspecified)?; + self.analyze_expr(&mut Context::default(), &order_by.expr, Type::Unspecified)?; } if !matches!(&query.projection.value, Value::Record(_) | Value::Id(_)) { @@ -563,7 +570,13 @@ impl<'a> Analysis<'a> { )); } - let project = self.analyze_expr(&query.projection, Type::Unspecified)?; + let mut ctx = Context { + analyzing_projection: true, + use_only_constant_expr: true, + ..Default::default() + }; + + let project = self.analyze_expr(&mut ctx, &query.projection, Type::Unspecified)?; if !matches!(&project, Type::Record(f) if !f.is_empty()) { return Err(AnalysisError::ExpectRecord( @@ -625,12 +638,18 @@ impl<'a> Analysis<'a> { } } - fn analyze_expr(&mut self, expr: &Expr, expect: Type) -> AnalysisResult { - self.analyze_value(&expr.attrs, &expr.value, expect) + fn analyze_expr( + &mut self, + ctx: &mut Context, + expr: &Expr, + expect: Type, + ) -> AnalysisResult { + self.analyze_value(ctx, &expr.attrs, &expr.value, expect) } fn analyze_value( &mut self, + ctx: &mut Context, attrs: &Attrs, value: &Value, mut expect: Type, @@ -647,6 +666,15 @@ impl<'a> Analysis<'a> { let tmp = mem::take(tpe); *tpe = tmp.check(attrs, expect)?; + if ctx.use_agg_func { + return Err(AnalysisError::UnallowedAggFuncUsageWithSrcField( + attrs.pos.line, + attrs.pos.col, + )); + } + + ctx.use_only_constant_expr = false; + Ok(tpe.clone()) } else { Err(AnalysisError::VariableUndeclared( @@ -660,7 +688,7 @@ impl<'a> Analysis<'a> { Value::Array(exprs) => { if matches!(expect, Type::Unspecified) { for expr in exprs { - expect = self.analyze_expr(expr, expect)?; + expect = self.analyze_expr(ctx, expr, expect)?; } return Ok(Type::Array(Box::new(expect))); @@ -669,7 +697,7 @@ impl<'a> Analysis<'a> { match expect { Type::Array(mut expect) => { for expr in exprs { - *expect = self.analyze_expr(expr, expect.as_ref().clone())?; + *expect = self.analyze_expr(ctx, expr, expect.as_ref().clone())?; } Ok(Type::Array(expect)) @@ -692,6 +720,7 @@ impl<'a> Analysis<'a> { record.insert( field.name.clone(), self.analyze_value( + ctx, &field.value.attrs, &field.value.value, Type::Unspecified, @@ -708,7 +737,7 @@ impl<'a> Analysis<'a> { if let Some(tpe) = types.remove(field.name.as_str()) { types.insert( field.name.clone(), - self.analyze_expr(&field.value, tpe)?, + self.analyze_expr(ctx, &field.value, tpe)?, ); } else { return Err(AnalysisError::FieldUndeclared( @@ -735,8 +764,29 @@ impl<'a> Analysis<'a> { Value::App(app) => { if let Some(tpe) = self.options.default_scope.entries.get(app.func.as_str()) - && let Type::App { args, result, aggregate } = tpe + && let Type::App { + args, + result, + aggregate, + } = tpe { + if !ctx.analyzing_projection && *aggregate { + return Err(AnalysisError::WrongAggFunUsage( + attrs.pos.line, + attrs.pos.col, + app.func.clone(), + )); + } + + if !ctx.use_only_constant_expr { + return Err(AnalysisError::UnallowedAggFuncUsageWithSrcField( + attrs.pos.line, + attrs.pos.col, + )); + } + + ctx.use_agg_func = true; + if args.len() != app.args.len() { return Err(AnalysisError::FunWrongArgumentCount( attrs.pos.line, @@ -748,7 +798,7 @@ impl<'a> Analysis<'a> { } for (arg, tpe) in app.args.iter().zip(args.iter().cloned()) { - self.analyze_expr(arg, tpe)?; + self.analyze_expr(ctx, arg, tpe)?; } // TODO - check if we are dealing with an aggregate function while not in a @@ -770,8 +820,8 @@ impl<'a> Analysis<'a> { Value::Binary(binary) => match binary.operator { Operator::Add | Operator::Sub | Operator::Mul | Operator::Div => { - self.analyze_expr(&binary.lhs, Type::Number)?; - self.analyze_expr(&binary.rhs, Type::Number)?; + self.analyze_expr(ctx, &binary.lhs, Type::Number)?; + self.analyze_expr(ctx, &binary.rhs, Type::Number)?; expect.check(attrs, Type::Number) } @@ -781,23 +831,26 @@ impl<'a> Analysis<'a> { | Operator::Lte | Operator::Gt | Operator::Gte => { - let lhs_expect = self.analyze_expr(&binary.lhs, Type::Unspecified)?; - let rhs_expect = self.analyze_expr(&binary.rhs, lhs_expect.clone())?; + let lhs_expect = self.analyze_expr(ctx, &binary.lhs, Type::Unspecified)?; + let rhs_expect = self.analyze_expr(ctx, &binary.rhs, lhs_expect.clone())?; // If the left side didn't have enough type information while the other did, // we replay another typecheck pass on the left side if the right side was conclusive if matches!(lhs_expect, Type::Unspecified) && !matches!(rhs_expect, Type::Unspecified) { - self.analyze_expr(&binary.lhs, rhs_expect)?; + self.analyze_expr(ctx, &binary.lhs, rhs_expect)?; } expect.check(attrs, Type::Bool) } Operator::Contains => { - let lhs_expect = - self.analyze_expr(&binary.lhs, Type::Array(Box::new(Type::Unspecified)))?; + let lhs_expect = self.analyze_expr( + ctx, + &binary.lhs, + Type::Array(Box::new(Type::Unspecified)), + )?; let lhs_assumption = match lhs_expect { Type::Array(inner) => *inner, @@ -810,22 +863,22 @@ impl<'a> Analysis<'a> { } }; - let rhs_expect = self.analyze_expr(&binary.rhs, lhs_assumption.clone())?; + let rhs_expect = self.analyze_expr(ctx, &binary.rhs, lhs_assumption.clone())?; // If the left side didn't have enough type information while the other did, // we replay another typecheck pass on the left side if the right side was conclusive if matches!(lhs_assumption, Type::Unspecified) && !matches!(rhs_expect, Type::Unspecified) { - self.analyze_expr(&binary.lhs, Type::Array(Box::new(rhs_expect)))?; + self.analyze_expr(ctx, &binary.lhs, Type::Array(Box::new(rhs_expect)))?; } expect.check(attrs, Type::Bool) } Operator::And | Operator::Or | Operator::Xor => { - self.analyze_expr(&binary.lhs, Type::Bool)?; - self.analyze_expr(&binary.rhs, Type::Bool)?; + self.analyze_expr(ctx, &binary.lhs, Type::Bool)?; + self.analyze_expr(ctx, &binary.rhs, Type::Bool)?; expect.check(attrs, Type::Bool) } @@ -854,19 +907,19 @@ impl<'a> Analysis<'a> { Value::Unary(unary) => match unary.operator { Operator::Add | Operator::Sub => { - self.analyze_expr(&unary.expr, Type::Number)?; + self.analyze_expr(ctx, &unary.expr, Type::Number)?; expect.check(attrs, Type::Number) } Operator::Not => { - self.analyze_expr(&unary.expr, Type::Bool)?; + self.analyze_expr(ctx, &unary.expr, Type::Bool)?; expect.check(attrs, Type::Bool) } _ => unreachable!(), }, - Value::Group(expr) => Ok(self.analyze_expr(expr.as_ref(), expect)?), + Value::Group(expr) => Ok(self.analyze_expr(ctx, expr.as_ref(), expect)?), } } diff --git a/src/error.rs b/src/error.rs index c8aec9f..07849d8 100644 --- a/src/error.rs +++ b/src/error.rs @@ -195,6 +195,12 @@ pub enum AnalysisError { #[error("{0}:{1}: function '{2}' requires {3} parameters but got {4}")] FunWrongArgumentCount(u32, u32, String, usize, usize), + + #[error("{0}:{1}: aggregate function '{2}' can only be used in a PROJECT INTO clause")] + WrongAggFunUsage(u32, u32, String), + + #[error("{0}:{1}: aggregate functions cannot be used with source-bound fields")] + UnallowedAggFuncUsageWithSrcField(u32, u32), } impl From for Error { From 65892eb18f36db6b3dc38587db0de751410096b3 Mon Sep 17 00:00:00 2001 From: YoEight Date: Tue, 6 Jan 2026 23:29:24 -0500 Subject: [PATCH 04/11] pending work --- src/analysis.rs | 3 + src/tests/analysis.rs | 9 ++ .../aggregate_with_sourced_bases_props.eql | 2 + ...aggregate_with_source_based_props.snap.new | 83 +++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 src/tests/resources/aggregate_with_sourced_bases_props.eql create mode 100644 src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap.new diff --git a/src/analysis.rs b/src/analysis.rs index d26fa00..09f90c9 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -486,6 +486,9 @@ impl Scope { } } +// TODO - find a better name +enum Frame {} + #[derive(Default)] struct Context { analyzing_projection: bool, diff --git a/src/tests/analysis.rs b/src/tests/analysis.rs index 3b13176..afc710a 100644 --- a/src/tests/analysis.rs +++ b/src/tests/analysis.rs @@ -73,3 +73,12 @@ fn test_analyze_valid_type_conversion_weird_case() { .unwrap(); insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); } + +#[test] +fn test_analyze_prevent_using_aggregate_with_source_based_props() { + let query = parse_query(include_str!( + "./resources/aggregate_with_sourced_bases_props.eql" + )) + .unwrap(); + insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); +} diff --git a/src/tests/resources/aggregate_with_sourced_bases_props.eql b/src/tests/resources/aggregate_with_sourced_bases_props.eql new file mode 100644 index 0000000..6c40c66 --- /dev/null +++ b/src/tests/resources/aggregate_with_sourced_bases_props.eql @@ -0,0 +1,2 @@ +FROM e IN events +PROJECT INTO { count: SUM(e.data.price), id: e.id } diff --git a/src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap.new b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap.new new file mode 100644 index 0000000..0425485 --- /dev/null +++ b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap.new @@ -0,0 +1,83 @@ +--- +source: src/tests/analysis.rs +assertion_line: 83 +expression: "query.run_static_analysis(&Default::default())" +--- +Ok: + attrs: + pos: + line: 1 + col: 1 + sources: + - binding: + name: e + pos: + line: 1 + col: 6 + kind: + Name: events + predicate: ~ + group_by: ~ + order_by: ~ + limit: ~ + projection: + attrs: + pos: + line: 2 + col: 14 + value: + Record: + - name: count + value: + attrs: + pos: + line: 2 + col: 23 + value: + App: + func: SUM + args: + - attrs: + pos: + line: 2 + col: 27 + value: + Access: + target: + attrs: + pos: + line: 2 + col: 27 + value: + Access: + target: + attrs: + pos: + line: 2 + col: 27 + value: + Id: e + field: data + field: price + - name: id + value: + attrs: + pos: + line: 2 + col: 46 + value: + Access: + target: + attrs: + pos: + line: 2 + col: 46 + value: + Id: e + field: id + distinct: false + meta: + project: + Record: + count: Number + id: String From 29c5464fc2065b7104d640aef09afe37c0553c4d Mon Sep 17 00:00:00 2001 From: YoEight Date: Thu, 8 Jan 2026 23:42:41 -0500 Subject: [PATCH 05/11] think I finally found the right approach to do this --- src/analysis.rs | 190 +++++++++++++++++++++++++++++------------------- 1 file changed, 116 insertions(+), 74 deletions(-) diff --git a/src/analysis.rs b/src/analysis.rs index 09f90c9..0828f15 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -8,7 +8,8 @@ use serde::Serialize; use unicase::Ascii; use crate::{ - Attrs, Expr, Query, Raw, Source, SourceKind, Type, Value, error::AnalysisError, token::Operator, + Attrs, Expr, Field, Query, Raw, Source, SourceKind, Type, Value, error::AnalysisError, + token::Operator, }; /// Represents the state of a query that has been statically analyzed. @@ -490,10 +491,9 @@ impl Scope { enum Frame {} #[derive(Default)] -struct Context { - analyzing_projection: bool, +struct CheckContext { use_agg_func: bool, - use_only_constant_expr: bool, + use_source_based: bool, } struct Analysis<'a> { @@ -538,7 +538,7 @@ impl<'a> Analysis<'a> { } if let Some(expr) = &query.predicate { - self.analyze_expr(&mut Context::default(), expr, Type::Bool)?; + self.analyze_expr(expr, Type::Bool)?; } if let Some(group_by) = &query.group_by { @@ -549,10 +549,10 @@ impl<'a> Analysis<'a> { )); } - self.analyze_expr(&mut Context::default(), &group_by.expr, Type::Unspecified)?; + self.analyze_expr(&group_by.expr, Type::Unspecified)?; if let Some(expr) = &group_by.predicate { - self.analyze_expr(&mut Context::default(), expr, Type::Bool)?; + self.analyze_expr(expr, Type::Bool)?; } } @@ -563,23 +563,10 @@ impl<'a> Analysis<'a> { order_by.expr.attrs.pos.col, )); } - self.analyze_expr(&mut Context::default(), &order_by.expr, Type::Unspecified)?; + self.analyze_expr(&order_by.expr, Type::Unspecified)?; } - if !matches!(&query.projection.value, Value::Record(_) | Value::Id(_)) { - return Err(AnalysisError::ExpectRecordLiteral( - query.projection.attrs.pos.line, - query.projection.attrs.pos.col, - )); - } - - let mut ctx = Context { - analyzing_projection: true, - use_only_constant_expr: true, - ..Default::default() - }; - - let project = self.analyze_expr(&mut ctx, &query.projection, Type::Unspecified)?; + let project = self.analyze_projection(&query.projection)?; if !matches!(&project, Type::Record(f) if !f.is_empty()) { return Err(AnalysisError::ExpectRecord( @@ -641,18 +628,95 @@ impl<'a> Analysis<'a> { } } - fn analyze_expr( + fn analyze_projection(&mut self, expr: &Expr) -> AnalysisResult { + match &expr.value { + Value::Record(record) => { + let tpe = self.analyze_expr(expr, Type::Unspecified)?; + self.check_projection_on_record(&mut CheckContext::default(), record.as_slice())?; + Ok(tpe) + } + + Value::Id(id) => { + if let Some(tpe) = self.scope.entries.get(id).cloned() { + if matches!(tpe, Type::Record(_)) { + Ok(tpe) + } else { + Err(AnalysisError::ExpectRecord( + expr.attrs.pos.line, + expr.attrs.pos.col, + tpe, + )) + } + } else { + Err(AnalysisError::VariableUndeclared( + expr.attrs.pos.line, + expr.attrs.pos.col, + id.clone(), + )) + } + } + + _ => Err(AnalysisError::ExpectRecord( + expr.attrs.pos.line, + expr.attrs.pos.col, + self.project_type(&expr.value), + )), + } + } + + fn check_projection_on_record( &mut self, - ctx: &mut Context, + ctx: &mut CheckContext, + record: &[Field], + ) -> AnalysisResult<()> { + for field in record { + self.check_projection_on_field(ctx, field)?; + } + + Ok(()) + } + + fn check_projection_on_field( + &mut self, + ctx: &mut CheckContext, + field: &Field, + ) -> AnalysisResult<()> { + self.check_projection_on_field_expr(ctx, &field.value) + } + + fn check_projection_on_field_expr( + &mut self, + ctx: &mut CheckContext, expr: &Expr, - expect: Type, - ) -> AnalysisResult { - self.analyze_value(ctx, &expr.attrs, &expr.value, expect) + ) -> AnalysisResult<()> { + match &expr.value { + Value::Number(_) | Value::String(_) | Value::Bool(_) => Ok(()), + Value::Id(id) => { + if ctx.use_agg_func && self.scope.entries.contains_key(id) { + Err(AnalysisError::UnallowedAggFuncUsageWithSrcField( + expr.attrs.pos.line, + expr.attrs.pos.col, + )) + } else { + Ok(()) + } + } + Value::Array(exprs) => todo!(), + Value::Record(fields) => todo!(), + Value::Access(access) => todo!(), + Value::App(app) => todo!(), + Value::Binary(binary) => todo!(), + Value::Unary(unary) => todo!(), + Value::Group(expr) => todo!(), + } + } + + fn analyze_expr(&mut self, expr: &Expr, expect: Type) -> AnalysisResult { + self.analyze_value(&expr.attrs, &expr.value, expect) } fn analyze_value( &mut self, - ctx: &mut Context, attrs: &Attrs, value: &Value, mut expect: Type, @@ -669,15 +733,6 @@ impl<'a> Analysis<'a> { let tmp = mem::take(tpe); *tpe = tmp.check(attrs, expect)?; - if ctx.use_agg_func { - return Err(AnalysisError::UnallowedAggFuncUsageWithSrcField( - attrs.pos.line, - attrs.pos.col, - )); - } - - ctx.use_only_constant_expr = false; - Ok(tpe.clone()) } else { Err(AnalysisError::VariableUndeclared( @@ -691,7 +746,7 @@ impl<'a> Analysis<'a> { Value::Array(exprs) => { if matches!(expect, Type::Unspecified) { for expr in exprs { - expect = self.analyze_expr(ctx, expr, expect)?; + expect = self.analyze_expr(expr, expect)?; } return Ok(Type::Array(Box::new(expect))); @@ -700,7 +755,7 @@ impl<'a> Analysis<'a> { match expect { Type::Array(mut expect) => { for expr in exprs { - *expect = self.analyze_expr(ctx, expr, expect.as_ref().clone())?; + *expect = self.analyze_expr(expr, expect.as_ref().clone())?; } Ok(Type::Array(expect)) @@ -723,7 +778,6 @@ impl<'a> Analysis<'a> { record.insert( field.name.clone(), self.analyze_value( - ctx, &field.value.attrs, &field.value.value, Type::Unspecified, @@ -740,7 +794,7 @@ impl<'a> Analysis<'a> { if let Some(tpe) = types.remove(field.name.as_str()) { types.insert( field.name.clone(), - self.analyze_expr(ctx, &field.value, tpe)?, + self.analyze_expr(&field.value, tpe)?, ); } else { return Err(AnalysisError::FieldUndeclared( @@ -773,35 +827,26 @@ impl<'a> Analysis<'a> { aggregate, } = tpe { - if !ctx.analyzing_projection && *aggregate { - return Err(AnalysisError::WrongAggFunUsage( + if args.len() != app.args.len() { + return Err(AnalysisError::FunWrongArgumentCount( attrs.pos.line, attrs.pos.col, app.func.clone(), + args.len(), + app.args.len(), )); } - if !ctx.use_only_constant_expr { - return Err(AnalysisError::UnallowedAggFuncUsageWithSrcField( - attrs.pos.line, - attrs.pos.col, - )); - } - - ctx.use_agg_func = true; - - if args.len() != app.args.len() { - return Err(AnalysisError::FunWrongArgumentCount( + if *aggregate { + return Err(AnalysisError::WrongAggFunUsage( attrs.pos.line, attrs.pos.col, app.func.clone(), - args.len(), - app.args.len(), )); } for (arg, tpe) in app.args.iter().zip(args.iter().cloned()) { - self.analyze_expr(ctx, arg, tpe)?; + self.analyze_expr(arg, tpe)?; } // TODO - check if we are dealing with an aggregate function while not in a @@ -823,8 +868,8 @@ impl<'a> Analysis<'a> { Value::Binary(binary) => match binary.operator { Operator::Add | Operator::Sub | Operator::Mul | Operator::Div => { - self.analyze_expr(ctx, &binary.lhs, Type::Number)?; - self.analyze_expr(ctx, &binary.rhs, Type::Number)?; + self.analyze_expr(&binary.lhs, Type::Number)?; + self.analyze_expr(&binary.rhs, Type::Number)?; expect.check(attrs, Type::Number) } @@ -834,26 +879,23 @@ impl<'a> Analysis<'a> { | Operator::Lte | Operator::Gt | Operator::Gte => { - let lhs_expect = self.analyze_expr(ctx, &binary.lhs, Type::Unspecified)?; - let rhs_expect = self.analyze_expr(ctx, &binary.rhs, lhs_expect.clone())?; + let lhs_expect = self.analyze_expr(&binary.lhs, Type::Unspecified)?; + let rhs_expect = self.analyze_expr(&binary.rhs, lhs_expect.clone())?; // If the left side didn't have enough type information while the other did, // we replay another typecheck pass on the left side if the right side was conclusive if matches!(lhs_expect, Type::Unspecified) && !matches!(rhs_expect, Type::Unspecified) { - self.analyze_expr(ctx, &binary.lhs, rhs_expect)?; + self.analyze_expr(&binary.lhs, rhs_expect)?; } expect.check(attrs, Type::Bool) } Operator::Contains => { - let lhs_expect = self.analyze_expr( - ctx, - &binary.lhs, - Type::Array(Box::new(Type::Unspecified)), - )?; + let lhs_expect = + self.analyze_expr(&binary.lhs, Type::Array(Box::new(Type::Unspecified)))?; let lhs_assumption = match lhs_expect { Type::Array(inner) => *inner, @@ -866,22 +908,22 @@ impl<'a> Analysis<'a> { } }; - let rhs_expect = self.analyze_expr(ctx, &binary.rhs, lhs_assumption.clone())?; + let rhs_expect = self.analyze_expr(&binary.rhs, lhs_assumption.clone())?; // If the left side didn't have enough type information while the other did, // we replay another typecheck pass on the left side if the right side was conclusive if matches!(lhs_assumption, Type::Unspecified) && !matches!(rhs_expect, Type::Unspecified) { - self.analyze_expr(ctx, &binary.lhs, Type::Array(Box::new(rhs_expect)))?; + self.analyze_expr(&binary.lhs, Type::Array(Box::new(rhs_expect)))?; } expect.check(attrs, Type::Bool) } Operator::And | Operator::Or | Operator::Xor => { - self.analyze_expr(ctx, &binary.lhs, Type::Bool)?; - self.analyze_expr(ctx, &binary.rhs, Type::Bool)?; + self.analyze_expr(&binary.lhs, Type::Bool)?; + self.analyze_expr(&binary.rhs, Type::Bool)?; expect.check(attrs, Type::Bool) } @@ -910,19 +952,19 @@ impl<'a> Analysis<'a> { Value::Unary(unary) => match unary.operator { Operator::Add | Operator::Sub => { - self.analyze_expr(ctx, &unary.expr, Type::Number)?; + self.analyze_expr(&unary.expr, Type::Number)?; expect.check(attrs, Type::Number) } Operator::Not => { - self.analyze_expr(ctx, &unary.expr, Type::Bool)?; + self.analyze_expr(&unary.expr, Type::Bool)?; expect.check(attrs, Type::Bool) } _ => unreachable!(), }, - Value::Group(expr) => Ok(self.analyze_expr(ctx, expr.as_ref(), expect)?), + Value::Group(expr) => Ok(self.analyze_expr(expr.as_ref(), expect)?), } } From 2c72632bd223d24002ea2ab8acad46b9da6132d2 Mon Sep 17 00:00:00 2001 From: YoEight Date: Fri, 9 Jan 2026 10:35:56 -0500 Subject: [PATCH 06/11] almost done. need to deny using aggregate function anywhere but PROJECT INTO --- src/analysis.rs | 86 +++++++++++++------ ...ing_aggregate_with_source_based_props.snap | 9 ++ ...aggregate_with_source_based_props.snap.new | 83 ------------------ 3 files changed, 71 insertions(+), 107 deletions(-) create mode 100644 src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap delete mode 100644 src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap.new diff --git a/src/analysis.rs b/src/analysis.rs index 0828f15..34662b1 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -487,9 +487,6 @@ impl Scope { } } -// TODO - find a better name -enum Frame {} - #[derive(Default)] struct CheckContext { use_agg_func: bool, @@ -691,23 +688,64 @@ impl<'a> Analysis<'a> { ) -> AnalysisResult<()> { match &expr.value { Value::Number(_) | Value::String(_) | Value::Bool(_) => Ok(()), + Value::Id(id) => { - if ctx.use_agg_func && self.scope.entries.contains_key(id) { - Err(AnalysisError::UnallowedAggFuncUsageWithSrcField( - expr.attrs.pos.line, - expr.attrs.pos.col, - )) - } else { - Ok(()) + if self.scope.entries.contains_key(id) { + if ctx.use_agg_func { + return Err(AnalysisError::UnallowedAggFuncUsageWithSrcField( + expr.attrs.pos.line, + expr.attrs.pos.col, + )); + } + + ctx.use_source_based = true; } + + Ok(()) } - Value::Array(exprs) => todo!(), - Value::Record(fields) => todo!(), - Value::Access(access) => todo!(), - Value::App(app) => todo!(), - Value::Binary(binary) => todo!(), - Value::Unary(unary) => todo!(), - Value::Group(expr) => todo!(), + + Value::Array(exprs) => { + for expr in exprs { + self.check_projection_on_field_expr(ctx, expr)?; + } + + Ok(()) + } + + Value::Record(fields) => { + for field in fields { + self.check_projection_on_field(ctx, field)?; + } + + Ok(()) + } + + Value::Access(access) => self.check_projection_on_field_expr(ctx, &access.target), + + Value::App(app) => { + if let Some(Type::App { aggregate, .. }) = + self.options.default_scope.entries.get(app.func.as_str()) + { + ctx.use_agg_func |= *aggregate; + + if ctx.use_agg_func && ctx.use_source_based { + return Err(AnalysisError::UnallowedAggFuncUsageWithSrcField( + expr.attrs.pos.line, + expr.attrs.pos.col, + )); + } + } + + Ok(()) + } + + Value::Binary(binary) => { + self.check_projection_on_field_expr(ctx, &binary.lhs)?; + self.check_projection_on_field_expr(ctx, &binary.rhs) + } + + Value::Unary(unary) => self.check_projection_on_field_expr(ctx, &unary.expr), + Value::Group(expr) => self.check_projection_on_field_expr(ctx, expr), } } @@ -837,13 +875,13 @@ impl<'a> Analysis<'a> { )); } - if *aggregate { - return Err(AnalysisError::WrongAggFunUsage( - attrs.pos.line, - attrs.pos.col, - app.func.clone(), - )); - } + // if *aggregate { + // return err(analysiserror::wrongaggfunusage( + // attrs.pos.line, + // attrs.pos.col, + // app.func.clone(), + // )); + // } for (arg, tpe) in app.args.iter().zip(args.iter().cloned()) { self.analyze_expr(arg, tpe)?; diff --git a/src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap new file mode 100644 index 0000000..68614c7 --- /dev/null +++ b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap @@ -0,0 +1,9 @@ +--- +source: src/tests/analysis.rs +expression: "query.run_static_analysis(&Default::default())" +--- +Err: + Analysis: + UnallowedAggFuncUsageWithSrcField: + - 2 + - 46 diff --git a/src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap.new b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap.new deleted file mode 100644 index 0425485..0000000 --- a/src/tests/snapshots/eventql_parser__tests__analysis__analyze_prevent_using_aggregate_with_source_based_props.snap.new +++ /dev/null @@ -1,83 +0,0 @@ ---- -source: src/tests/analysis.rs -assertion_line: 83 -expression: "query.run_static_analysis(&Default::default())" ---- -Ok: - attrs: - pos: - line: 1 - col: 1 - sources: - - binding: - name: e - pos: - line: 1 - col: 6 - kind: - Name: events - predicate: ~ - group_by: ~ - order_by: ~ - limit: ~ - projection: - attrs: - pos: - line: 2 - col: 14 - value: - Record: - - name: count - value: - attrs: - pos: - line: 2 - col: 23 - value: - App: - func: SUM - args: - - attrs: - pos: - line: 2 - col: 27 - value: - Access: - target: - attrs: - pos: - line: 2 - col: 27 - value: - Access: - target: - attrs: - pos: - line: 2 - col: 27 - value: - Id: e - field: data - field: price - - name: id - value: - attrs: - pos: - line: 2 - col: 46 - value: - Access: - target: - attrs: - pos: - line: 2 - col: 46 - value: - Id: e - field: id - distinct: false - meta: - project: - Record: - count: Number - id: String From 6625883f990cddc26a51aaad211fd8079848b4f2 Mon Sep 17 00:00:00 2001 From: YoEight Date: Fri, 9 Jan 2026 15:25:45 -0500 Subject: [PATCH 07/11] done --- src/analysis.rs | 235 +++++++++++------- src/error.rs | 3 + src/tests/analysis.rs | 12 + .../resources/reject_agg_in_predicate.eql | 3 + src/tests/resources/valid_agg_usage.eql | 6 + ...ysis__analyze_reject_agg_in_predicate.snap | 10 + ...ts__analysis__analyze_valid_agg_usage.snap | 85 +++++++ 7 files changed, 267 insertions(+), 87 deletions(-) create mode 100644 src/tests/resources/reject_agg_in_predicate.eql create mode 100644 src/tests/resources/valid_agg_usage.eql create mode 100644 src/tests/snapshots/eventql_parser__tests__analysis__analyze_reject_agg_in_predicate.snap create mode 100644 src/tests/snapshots/eventql_parser__tests__analysis__analyze_valid_agg_usage.snap diff --git a/src/analysis.rs b/src/analysis.rs index 34662b1..2149c74 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -176,7 +176,7 @@ impl Default for AnalysisOptions { ( "RAND".to_owned(), Type::App { - args: vec![Type::Number], + args: vec![], result: Box::new(Type::Number), aggregate: false, }, @@ -416,8 +416,8 @@ impl Default for AnalysisOptions { ( "UNIQUE".to_owned(), Type::App { - args: vec![Type::Number], - result: Box::new(Type::Number), + args: vec![Type::Unspecified], + result: Box::new(Type::Unspecified), aggregate: true, }, ), @@ -493,6 +493,11 @@ struct CheckContext { use_source_based: bool, } +#[derive(Default)] +struct AnalysisContext { + allow_agg_func: bool, +} + struct Analysis<'a> { options: &'a AnalysisOptions, prev_scopes: Vec, @@ -529,13 +534,14 @@ impl<'a> Analysis<'a> { self.enter_scope(); let mut sources = Vec::with_capacity(query.sources.len()); + let mut ctx = AnalysisContext::default(); for source in query.sources { sources.push(self.analyze_source(source)?); } if let Some(expr) = &query.predicate { - self.analyze_expr(expr, Type::Bool)?; + self.analyze_expr(&ctx, expr, Type::Bool)?; } if let Some(group_by) = &query.group_by { @@ -546,10 +552,10 @@ impl<'a> Analysis<'a> { )); } - self.analyze_expr(&group_by.expr, Type::Unspecified)?; + self.analyze_expr(&ctx, &group_by.expr, Type::Unspecified)?; if let Some(expr) = &group_by.predicate { - self.analyze_expr(expr, Type::Bool)?; + self.analyze_expr(&ctx, expr, Type::Bool)?; } } @@ -560,19 +566,10 @@ impl<'a> Analysis<'a> { order_by.expr.attrs.pos.col, )); } - self.analyze_expr(&order_by.expr, Type::Unspecified)?; - } - - let project = self.analyze_projection(&query.projection)?; - - if !matches!(&project, Type::Record(f) if !f.is_empty()) { - return Err(AnalysisError::ExpectRecord( - query.projection.attrs.pos.line, - query.projection.attrs.pos.col, - project, - )); + self.analyze_expr(&ctx, &order_by.expr, Type::Unspecified)?; } + let project = self.analyze_projection(&mut ctx, &query.projection)?; let scope = self.exit_scope(); Ok(Query { @@ -625,17 +622,29 @@ impl<'a> Analysis<'a> { } } - fn analyze_projection(&mut self, expr: &Expr) -> AnalysisResult { + fn analyze_projection( + &mut self, + ctx: &mut AnalysisContext, + expr: &Expr, + ) -> AnalysisResult { match &expr.value { Value::Record(record) => { - let tpe = self.analyze_expr(expr, Type::Unspecified)?; + if record.is_empty() { + return Err(AnalysisError::EmptyRecord( + expr.attrs.pos.line, + expr.attrs.pos.col, + )); + } + + ctx.allow_agg_func = true; + let tpe = self.analyze_expr(ctx, expr, Type::Unspecified)?; self.check_projection_on_record(&mut CheckContext::default(), record.as_slice())?; Ok(tpe) } Value::Id(id) => { if let Some(tpe) = self.scope.entries.get(id).cloned() { - if matches!(tpe, Type::Record(_)) { + if matches!(&tpe, Type::Record(f) if !f.is_empty()) { Ok(tpe) } else { Err(AnalysisError::ExpectRecord( @@ -734,6 +743,10 @@ impl<'a> Analysis<'a> { expr.attrs.pos.col, )); } + + for arg in &app.args { + self.invalidate_agg_func_usage(arg)?; + } } Ok(()) @@ -749,33 +762,82 @@ impl<'a> Analysis<'a> { } } - fn analyze_expr(&mut self, expr: &Expr, expect: Type) -> AnalysisResult { - self.analyze_value(&expr.attrs, &expr.value, expect) + fn invalidate_agg_func_usage(&mut self, expr: &Expr) -> AnalysisResult<()> { + match &expr.value { + Value::Number(_) + | Value::String(_) + | Value::Bool(_) + | Value::Id(_) + | Value::Access(_) => Ok(()), + + Value::Array(exprs) => { + for expr in exprs { + self.invalidate_agg_func_usage(expr)?; + } + + Ok(()) + } + + Value::Record(fields) => { + for field in fields { + self.invalidate_agg_func_usage(&field.value)?; + } + + Ok(()) + } + + Value::App(app) => { + if let Some(Type::App { aggregate, .. }) = + self.options.default_scope.entries.get(&app.func) + && *aggregate + { + return Err(AnalysisError::WrongAggFunUsage( + expr.attrs.pos.line, + expr.attrs.pos.col, + app.func.clone(), + )); + } + + for arg in &app.args { + self.invalidate_agg_func_usage(arg)?; + } + + Ok(()) + } + + Value::Binary(binary) => { + self.invalidate_agg_func_usage(&binary.lhs)?; + self.invalidate_agg_func_usage(&binary.rhs) + } + + Value::Unary(unary) => self.invalidate_agg_func_usage(&unary.expr), + Value::Group(expr) => self.invalidate_agg_func_usage(expr), + } } - fn analyze_value( + fn analyze_expr( &mut self, - attrs: &Attrs, - value: &Value, + ctx: &AnalysisContext, + expr: &Expr, mut expect: Type, ) -> AnalysisResult { - match value { - Value::Number(_) => expect.check(attrs, Type::Number), - Value::String(_) => expect.check(attrs, Type::String), - Value::Bool(_) => expect.check(attrs, Type::Bool), + match &expr.value { + Value::Number(_) => expect.check(&expr.attrs, Type::Number), + Value::String(_) => expect.check(&expr.attrs, Type::String), + Value::Bool(_) => expect.check(&expr.attrs, Type::Bool), Value::Id(id) => { if let Some(tpe) = self.options.default_scope.entries.get(id) { - expect.check(attrs, tpe.clone()) + expect.check(&expr.attrs, tpe.clone()) } else if let Some(tpe) = self.scope.entries.get_mut(id.as_str()) { let tmp = mem::take(tpe); - *tpe = tmp.check(attrs, expect)?; + *tpe = tmp.check(&expr.attrs, expect)?; Ok(tpe.clone()) } else { Err(AnalysisError::VariableUndeclared( - attrs.pos.line, - attrs.pos.col, + expr.attrs.pos.line, + expr.attrs.pos.col, id.to_owned(), )) } @@ -784,7 +846,7 @@ impl<'a> Analysis<'a> { Value::Array(exprs) => { if matches!(expect, Type::Unspecified) { for expr in exprs { - expect = self.analyze_expr(expr, expect)?; + expect = self.analyze_expr(ctx, expr, expect)?; } return Ok(Type::Array(Box::new(expect))); @@ -793,17 +855,17 @@ impl<'a> Analysis<'a> { match expect { Type::Array(mut expect) => { for expr in exprs { - *expect = self.analyze_expr(expr, expect.as_ref().clone())?; + *expect = self.analyze_expr(ctx, expr, expect.as_ref().clone())?; } Ok(Type::Array(expect)) } expect => Err(AnalysisError::TypeMismatch( - attrs.pos.line, - attrs.pos.col, + expr.attrs.pos.line, + expr.attrs.pos.col, expect, - self.project_type(value), + self.project_type(&expr.value), )), } } @@ -815,11 +877,7 @@ impl<'a> Analysis<'a> { for field in fields { record.insert( field.name.clone(), - self.analyze_value( - &field.value.attrs, - &field.value.value, - Type::Unspecified, - )?, + self.analyze_expr(ctx, &field.value, Type::Unspecified)?, ); } @@ -832,12 +890,12 @@ impl<'a> Analysis<'a> { if let Some(tpe) = types.remove(field.name.as_str()) { types.insert( field.name.clone(), - self.analyze_expr(&field.value, tpe)?, + self.analyze_expr(ctx, &field.value, tpe)?, ); } else { return Err(AnalysisError::FieldUndeclared( - attrs.pos.line, - attrs.pos.col, + expr.attrs.pos.line, + expr.attrs.pos.col, field.name.clone(), )); } @@ -847,15 +905,15 @@ impl<'a> Analysis<'a> { } expect => Err(AnalysisError::TypeMismatch( - attrs.pos.line, - attrs.pos.col, + expr.attrs.pos.line, + expr.attrs.pos.col, expect, - self.project_type(value), + self.project_type(&expr.value), )), } } - this @ Value::Access(_) => Ok(self.analyze_access(attrs, this, expect)?), + this @ Value::Access(_) => Ok(self.analyze_access(&expr.attrs, this, expect)?), Value::App(app) => { if let Some(tpe) = self.options.default_scope.entries.get(app.func.as_str()) @@ -867,24 +925,24 @@ impl<'a> Analysis<'a> { { if args.len() != app.args.len() { return Err(AnalysisError::FunWrongArgumentCount( - attrs.pos.line, - attrs.pos.col, + expr.attrs.pos.line, + expr.attrs.pos.col, app.func.clone(), args.len(), app.args.len(), )); } - // if *aggregate { - // return err(analysiserror::wrongaggfunusage( - // attrs.pos.line, - // attrs.pos.col, - // app.func.clone(), - // )); - // } + if *aggregate && !ctx.allow_agg_func { + return Err(AnalysisError::WrongAggFunUsage( + expr.attrs.pos.line, + expr.attrs.pos.col, + app.func.clone(), + )); + } for (arg, tpe) in app.args.iter().zip(args.iter().cloned()) { - self.analyze_expr(arg, tpe)?; + self.analyze_expr(ctx, arg, tpe)?; } // TODO - check if we are dealing with an aggregate function while not in a @@ -893,12 +951,12 @@ impl<'a> Analysis<'a> { if matches!(expect, Type::Unspecified) { Ok(result.as_ref().clone()) } else { - expect.check(attrs, result.as_ref().clone()) + expect.check(&expr.attrs, result.as_ref().clone()) } } else { Err(AnalysisError::FuncUndeclared( - attrs.pos.line, - attrs.pos.col, + expr.attrs.pos.line, + expr.attrs.pos.col, app.func.clone(), )) } @@ -906,9 +964,9 @@ impl<'a> Analysis<'a> { Value::Binary(binary) => match binary.operator { Operator::Add | Operator::Sub | Operator::Mul | Operator::Div => { - self.analyze_expr(&binary.lhs, Type::Number)?; - self.analyze_expr(&binary.rhs, Type::Number)?; - expect.check(attrs, Type::Number) + self.analyze_expr(ctx, &binary.lhs, Type::Number)?; + self.analyze_expr(ctx, &binary.rhs, Type::Number)?; + expect.check(&expr.attrs, Type::Number) } Operator::Eq @@ -917,53 +975,56 @@ impl<'a> Analysis<'a> { | Operator::Lte | Operator::Gt | Operator::Gte => { - let lhs_expect = self.analyze_expr(&binary.lhs, Type::Unspecified)?; - let rhs_expect = self.analyze_expr(&binary.rhs, lhs_expect.clone())?; + let lhs_expect = self.analyze_expr(ctx, &binary.lhs, Type::Unspecified)?; + let rhs_expect = self.analyze_expr(ctx, &binary.rhs, lhs_expect.clone())?; // If the left side didn't have enough type information while the other did, // we replay another typecheck pass on the left side if the right side was conclusive if matches!(lhs_expect, Type::Unspecified) && !matches!(rhs_expect, Type::Unspecified) { - self.analyze_expr(&binary.lhs, rhs_expect)?; + self.analyze_expr(ctx, &binary.lhs, rhs_expect)?; } - expect.check(attrs, Type::Bool) + expect.check(&expr.attrs, Type::Bool) } Operator::Contains => { - let lhs_expect = - self.analyze_expr(&binary.lhs, Type::Array(Box::new(Type::Unspecified)))?; + let lhs_expect = self.analyze_expr( + ctx, + &binary.lhs, + Type::Array(Box::new(Type::Unspecified)), + )?; let lhs_assumption = match lhs_expect { Type::Array(inner) => *inner, other => { return Err(AnalysisError::ExpectArray( - attrs.pos.line, - attrs.pos.col, + expr.attrs.pos.line, + expr.attrs.pos.col, other, )); } }; - let rhs_expect = self.analyze_expr(&binary.rhs, lhs_assumption.clone())?; + let rhs_expect = self.analyze_expr(ctx, &binary.rhs, lhs_assumption.clone())?; // If the left side didn't have enough type information while the other did, // we replay another typecheck pass on the left side if the right side was conclusive if matches!(lhs_assumption, Type::Unspecified) && !matches!(rhs_expect, Type::Unspecified) { - self.analyze_expr(&binary.lhs, Type::Array(Box::new(rhs_expect)))?; + self.analyze_expr(ctx, &binary.lhs, Type::Array(Box::new(rhs_expect)))?; } - expect.check(attrs, Type::Bool) + expect.check(&expr.attrs, Type::Bool) } Operator::And | Operator::Or | Operator::Xor => { - self.analyze_expr(&binary.lhs, Type::Bool)?; - self.analyze_expr(&binary.rhs, Type::Bool)?; + self.analyze_expr(ctx, &binary.lhs, Type::Bool)?; + self.analyze_expr(ctx, &binary.rhs, Type::Bool)?; - expect.check(attrs, Type::Bool) + expect.check(&expr.attrs, Type::Bool) } Operator::As => { @@ -973,8 +1034,8 @@ impl<'a> Analysis<'a> { return Ok(tpe); } else { return Err(AnalysisError::UnsupportedCustomType( - attrs.pos.line, - attrs.pos.col, + expr.attrs.pos.line, + expr.attrs.pos.col, name.clone(), )); } @@ -990,19 +1051,19 @@ impl<'a> Analysis<'a> { Value::Unary(unary) => match unary.operator { Operator::Add | Operator::Sub => { - self.analyze_expr(&unary.expr, Type::Number)?; - expect.check(attrs, Type::Number) + self.analyze_expr(ctx, &unary.expr, Type::Number)?; + expect.check(&expr.attrs, Type::Number) } Operator::Not => { - self.analyze_expr(&unary.expr, Type::Bool)?; - expect.check(attrs, Type::Bool) + self.analyze_expr(ctx, &unary.expr, Type::Bool)?; + expect.check(&expr.attrs, Type::Bool) } _ => unreachable!(), }, - Value::Group(expr) => Ok(self.analyze_expr(expr.as_ref(), expect)?), + Value::Group(expr) => Ok(self.analyze_expr(ctx, expr.as_ref(), expect)?), } } diff --git a/src/error.rs b/src/error.rs index 07849d8..feb0de4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -201,6 +201,9 @@ pub enum AnalysisError { #[error("{0}:{1}: aggregate functions cannot be used with source-bound fields")] UnallowedAggFuncUsageWithSrcField(u32, u32), + + #[error("{0}:{1}: unexpected empty record")] + EmptyRecord(u32, u32), } impl From for Error { diff --git a/src/tests/analysis.rs b/src/tests/analysis.rs index afc710a..e415091 100644 --- a/src/tests/analysis.rs +++ b/src/tests/analysis.rs @@ -82,3 +82,15 @@ fn test_analyze_prevent_using_aggregate_with_source_based_props() { .unwrap(); insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); } + +#[test] +fn test_analyze_valid_agg_usage() { + let query = parse_query(include_str!("./resources/valid_agg_usage.eql")).unwrap(); + insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); +} + +#[test] +fn test_analyze_reject_agg_in_predicate() { + let query = parse_query(include_str!("./resources/reject_agg_in_predicate.eql")).unwrap(); + insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); +} diff --git a/src/tests/resources/reject_agg_in_predicate.eql b/src/tests/resources/reject_agg_in_predicate.eql new file mode 100644 index 0000000..34b8190 --- /dev/null +++ b/src/tests/resources/reject_agg_in_predicate.eql @@ -0,0 +1,3 @@ +FROM e IN events +WHERE 3 > COUNT() +PROJECT INTO e diff --git a/src/tests/resources/valid_agg_usage.eql b/src/tests/resources/valid_agg_usage.eql new file mode 100644 index 0000000..b30d89a --- /dev/null +++ b/src/tests/resources/valid_agg_usage.eql @@ -0,0 +1,6 @@ +FROM e IN events +PROJECT INTO { + sum: SUM(e.data.count), + label: "everything summed", + randomvalue: RAND() +} diff --git a/src/tests/snapshots/eventql_parser__tests__analysis__analyze_reject_agg_in_predicate.snap b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_reject_agg_in_predicate.snap new file mode 100644 index 0000000..5e911e7 --- /dev/null +++ b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_reject_agg_in_predicate.snap @@ -0,0 +1,10 @@ +--- +source: src/tests/analysis.rs +expression: "query.run_static_analysis(&Default::default())" +--- +Err: + Analysis: + WrongAggFunUsage: + - 2 + - 11 + - COUNT diff --git a/src/tests/snapshots/eventql_parser__tests__analysis__analyze_valid_agg_usage.snap b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_valid_agg_usage.snap new file mode 100644 index 0000000..6390462 --- /dev/null +++ b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_valid_agg_usage.snap @@ -0,0 +1,85 @@ +--- +source: src/tests/analysis.rs +expression: "query.run_static_analysis(&Default::default())" +--- +Ok: + attrs: + pos: + line: 1 + col: 1 + sources: + - binding: + name: e + pos: + line: 1 + col: 6 + kind: + Name: events + predicate: ~ + group_by: ~ + order_by: ~ + limit: ~ + projection: + attrs: + pos: + line: 2 + col: 14 + value: + Record: + - name: sum + value: + attrs: + pos: + line: 3 + col: 7 + value: + App: + func: SUM + args: + - attrs: + pos: + line: 3 + col: 11 + value: + Access: + target: + attrs: + pos: + line: 3 + col: 11 + value: + Access: + target: + attrs: + pos: + line: 3 + col: 11 + value: + Id: e + field: data + field: count + - name: label + value: + attrs: + pos: + line: 4 + col: 9 + value: + String: everything summed + - name: randomvalue + value: + attrs: + pos: + line: 5 + col: 15 + value: + App: + func: RAND + args: [] + distinct: false + meta: + project: + Record: + label: String + randomvalue: Number + sum: Number From a43e1c317a5a225c75f1433b66d922b7235b8bab Mon Sep 17 00:00:00 2001 From: YoEight Date: Fri, 9 Jan 2026 15:44:41 -0500 Subject: [PATCH 08/11] add documentation --- src/analysis.rs | 3 ++ src/ast.rs | 9 ++++++ src/error.rs | 76 ++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/analysis.rs b/src/analysis.rs index 2149c74..8a426ef 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -451,6 +451,9 @@ impl Default for AnalysisOptions { /// - Types match expected types in expressions and operations /// - Field accesses are valid for their record types /// - Function calls have the correct argument types +/// - Aggregate functions are only used in PROJECT INTO clauses +/// - Aggregate functions are not mixed with source-bound fields in projections +/// - Record literals are non-empty in projection contexts /// /// # Arguments /// diff --git a/src/ast.rs b/src/ast.rs index 14e6ff1..9afc491 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -540,6 +540,15 @@ impl Query { /// analysis on the query, converting it from a raw (untyped) query to a /// typed query. /// + /// The analysis validates: + /// - Variable declarations and scoping + /// - Type compatibility in expressions and operations + /// - Valid field accesses on record types + /// - Correct function argument types and counts + /// - Aggregate function usage restrictions (only in PROJECT INTO) + /// - No mixing of aggregate functions with source-bound fields + /// - Non-empty record literals in projections + /// /// # Arguments /// /// * `options` - Configuration containing type information and default scope diff --git a/src/error.rs b/src/error.rs index feb0de4..862b6c6 100644 --- a/src/error.rs +++ b/src/error.rs @@ -184,7 +184,7 @@ pub enum AnalysisError { /// Fields: `(line, column)` /// /// This occurs when a record literal is required, such as in the - /// SELECT projection clause. + /// PROJECT INTO clause. #[error("{0}:{1}: expected a record")] ExpectRecordLiteral(u32, u32), @@ -193,15 +193,89 @@ pub enum AnalysisError { #[error("{0}:{1}: unsupported custom type '{2}'")] UnsupportedCustomType(u32, u32, String), + /// A function was called with the wrong number of arguments. + /// + /// Fields: `(line, column, function_name, expected_count, actual_count)` + /// + /// This occurs when calling a function with a different number of arguments + /// than what the function signature requires. #[error("{0}:{1}: function '{2}' requires {3} parameters but got {4}")] FunWrongArgumentCount(u32, u32, String, usize, usize), + /// An aggregate function was used outside of a PROJECT INTO clause. + /// + /// Fields: `(line, column, function_name)` + /// + /// This occurs when an aggregate function (e.g., SUM, COUNT, AVG) is used + /// in a context where aggregation is not allowed, such as in WHERE, GROUP BY, + /// or ORDER BY clauses. Aggregate functions can only be used in the PROJECT INTO + /// clause to compute aggregated values over groups of events. + /// + /// # Example + /// + /// Invalid usage: + /// ```eql + /// FROM e IN events + /// WHERE COUNT() > 5 // Error: aggregate function in WHERE clause + /// PROJECT INTO e + /// ``` + /// + /// Valid usage: + /// ```eql + /// FROM e IN events + /// PROJECT INTO { total: COUNT() } + /// ``` #[error("{0}:{1}: aggregate function '{2}' can only be used in a PROJECT INTO clause")] WrongAggFunUsage(u32, u32, String), + /// An aggregate function was used together with source-bound fields. + /// + /// Fields: `(line, column)` + /// + /// This occurs when attempting to mix aggregate functions with fields that are + /// bound to source events within the same projection field. Aggregate functions + /// operate on groups of events, while source-bound fields refer to individual + /// event properties. These cannot be mixed in a single field expression. + /// + /// # Example + /// + /// Invalid usage: + /// ```eql + /// FROM e IN events + /// // Error: mixing aggregate (SUM) with source field (e.id) + /// PROJECT INTO { count: SUM(e.data.price), id: e.id } + /// ``` + /// + /// Valid usage: + /// ```eql + /// FROM e IN events + /// // OK: only using aggregate functions and constants + /// PROJECT INTO { sum: SUM(e.data.price), label: "total" } + /// ``` #[error("{0}:{1}: aggregate functions cannot be used with source-bound fields")] UnallowedAggFuncUsageWithSrcField(u32, u32), + /// An empty record literal was used in a context where it is not allowed. + /// + /// Fields: `(line, column)` + /// + /// This occurs when using an empty record `{}` as a projection, which would + /// result in a query that produces no output fields. Projections must contain + /// at least one field. + /// + /// # Example + /// + /// Invalid usage: + /// ```eql + /// FROM e IN events + /// PROJECT INTO {} // Error: empty record + /// ``` + /// + /// Valid usage: + /// ```eql + /// FROM e IN events + /// PROJECT INTO { id: e.id } + /// ``` #[error("{0}:{1}: unexpected empty record")] EmptyRecord(u32, u32), } From a82b77cdd1853c7ce7c52ac44f564301b89cae09 Mon Sep 17 00:00:00 2001 From: YoEight Date: Fri, 9 Jan 2026 15:59:58 -0500 Subject: [PATCH 09/11] add another test --- src/analysis.rs | 18 +++++++++++++++--- src/error.rs | 3 +++ src/tests/analysis.rs | 6 ++++++ .../resources/agg_must_use_source_bound.eql | 2 ++ ...sis__analyze_agg_must_use_source_bound.snap | 9 +++++++++ 5 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 src/tests/resources/agg_must_use_source_bound.eql create mode 100644 src/tests/snapshots/eventql_parser__tests__analysis__analyze_agg_must_use_source_bound.snap diff --git a/src/analysis.rs b/src/analysis.rs index 8a426ef..decc382 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -748,6 +748,10 @@ impl<'a> Analysis<'a> { } for arg in &app.args { + if *aggregate { + self.ensure_agg_param_is_source_bound(arg)?; + } + self.invalidate_agg_func_usage(arg)?; } } @@ -765,6 +769,17 @@ impl<'a> Analysis<'a> { } } + fn ensure_agg_param_is_source_bound(&mut self, expr: &Expr) -> AnalysisResult<()> { + match &expr.value { + Value::Id(id) if !self.options.default_scope.entries.contains_key(id) => Ok(()), + Value::Access(access) => self.ensure_agg_param_is_source_bound(&access.target), + _ => Err(AnalysisError::ExpectSourceBoundProperty( + expr.attrs.pos.line, + expr.attrs.pos.col, + )), + } + } + fn invalidate_agg_func_usage(&mut self, expr: &Expr) -> AnalysisResult<()> { match &expr.value { Value::Number(_) @@ -948,9 +963,6 @@ impl<'a> Analysis<'a> { self.analyze_expr(ctx, arg, tpe)?; } - // TODO - check if we are dealing with an aggregate function while not in a - // projection expression. - if matches!(expect, Type::Unspecified) { Ok(result.as_ref().clone()) } else { diff --git a/src/error.rs b/src/error.rs index 862b6c6..c9b32dc 100644 --- a/src/error.rs +++ b/src/error.rs @@ -278,6 +278,9 @@ pub enum AnalysisError { /// ``` #[error("{0}:{1}: unexpected empty record")] EmptyRecord(u32, u32), + + #[error("{0}:{1}: aggregate functions arguments must be source-bound fields")] + ExpectSourceBoundProperty(u32, u32), } impl From for Error { diff --git a/src/tests/analysis.rs b/src/tests/analysis.rs index e415091..34be555 100644 --- a/src/tests/analysis.rs +++ b/src/tests/analysis.rs @@ -94,3 +94,9 @@ fn test_analyze_reject_agg_in_predicate() { let query = parse_query(include_str!("./resources/reject_agg_in_predicate.eql")).unwrap(); insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); } + +#[test] +fn test_analyze_agg_must_use_source_bound() { + let query = parse_query(include_str!("./resources/agg_must_use_source_bound.eql")).unwrap(); + insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); +} diff --git a/src/tests/resources/agg_must_use_source_bound.eql b/src/tests/resources/agg_must_use_source_bound.eql new file mode 100644 index 0000000..00e28a9 --- /dev/null +++ b/src/tests/resources/agg_must_use_source_bound.eql @@ -0,0 +1,2 @@ +FROM e IN events +PROJECT INTO { sum: SUM(RAND()) } diff --git a/src/tests/snapshots/eventql_parser__tests__analysis__analyze_agg_must_use_source_bound.snap b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_agg_must_use_source_bound.snap new file mode 100644 index 0000000..aa725d9 --- /dev/null +++ b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_agg_must_use_source_bound.snap @@ -0,0 +1,9 @@ +--- +source: src/tests/analysis.rs +expression: "query.run_static_analysis(&Default::default())" +--- +Err: + Analysis: + ExpectSourceBoundProperty: + - 2 + - 25 From f27826b5e82ea530369dc2467255c38d7788b23a Mon Sep 17 00:00:00 2001 From: YoEight Date: Fri, 9 Jan 2026 16:03:54 -0500 Subject: [PATCH 10/11] complete --- src/analysis.rs | 1 + src/ast.rs | 1 + src/error.rs | 24 ++++++++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/src/analysis.rs b/src/analysis.rs index decc382..de30fbb 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -453,6 +453,7 @@ impl Default for AnalysisOptions { /// - Function calls have the correct argument types /// - Aggregate functions are only used in PROJECT INTO clauses /// - Aggregate functions are not mixed with source-bound fields in projections +/// - Aggregate function arguments are source-bound fields (not constants or function results) /// - Record literals are non-empty in projection contexts /// /// # Arguments diff --git a/src/ast.rs b/src/ast.rs index 9afc491..f6f7dcc 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -547,6 +547,7 @@ impl Query { /// - Correct function argument types and counts /// - Aggregate function usage restrictions (only in PROJECT INTO) /// - No mixing of aggregate functions with source-bound fields + /// - Aggregate function arguments are source-bound fields /// - Non-empty record literals in projections /// /// # Arguments diff --git a/src/error.rs b/src/error.rs index c9b32dc..2ce1d5e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -279,6 +279,30 @@ pub enum AnalysisError { #[error("{0}:{1}: unexpected empty record")] EmptyRecord(u32, u32), + /// An aggregate function was called with an argument that is not a source-bound field. + /// + /// Fields: `(line, column)` + /// + /// This occurs when an aggregate function (e.g., SUM, COUNT, AVG) is called with + /// an argument that is not derived from source event properties. Aggregate functions + /// must operate on fields that come from the source events being queried, not on + /// constants, literals, or results from other functions. + /// + /// # Example + /// + /// Invalid usage: + /// ```eql + /// FROM e IN events + /// // Error: RAND() is constant value + /// PROJECT INTO { sum: SUM(RAND()) } + /// ``` + /// + /// Valid usage: + /// ```eql + /// FROM e IN events + /// PROJECT INTO { sum: SUM(e.data.price) } + /// -- OK: e.data.price is a source-bound field + /// ``` #[error("{0}:{1}: aggregate functions arguments must be source-bound fields")] ExpectSourceBoundProperty(u32, u32), } From a5c071bd0a8a231a1631a1fcef6fd11d1febc316 Mon Sep 17 00:00:00 2001 From: YoEight Date: Fri, 9 Jan 2026 16:05:02 -0500 Subject: [PATCH 11/11] fixup --- src/error.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/error.rs b/src/error.rs index 2ce1d5e..34d2f11 100644 --- a/src/error.rs +++ b/src/error.rs @@ -249,7 +249,6 @@ pub enum AnalysisError { /// Valid usage: /// ```eql /// FROM e IN events - /// // OK: only using aggregate functions and constants /// PROJECT INTO { sum: SUM(e.data.price), label: "total" } /// ``` #[error("{0}:{1}: aggregate functions cannot be used with source-bound fields")] @@ -301,7 +300,6 @@ pub enum AnalysisError { /// ```eql /// FROM e IN events /// PROJECT INTO { sum: SUM(e.data.price) } - /// -- OK: e.data.price is a source-bound field /// ``` #[error("{0}:{1}: aggregate functions arguments must be source-bound fields")] ExpectSourceBoundProperty(u32, u32),