diff --git a/src/analysis.rs b/src/analysis.rs index fa95a8c..d2662d1 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -653,6 +653,9 @@ impl<'a> Analysis<'a> { if let Some(expr) = &group_by.predicate { self.analyze_expr(&mut ctx, expr, Type::Bool)?; } + + ctx.allow_agg_func = true; + ctx.use_agg_funcs = true; } let project = self.analyze_projection(&mut ctx, &query.projection)?; @@ -742,7 +745,12 @@ impl<'a> Analysis<'a> { 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())?; + let mut chk_ctx = CheckContext { + use_agg_func: ctx.use_agg_funcs, + ..Default::default() + }; + + self.check_projection_on_record(&mut chk_ctx, record.as_slice())?; Ok(tpe) } @@ -752,7 +760,12 @@ impl<'a> Analysis<'a> { let tpe = self.analyze_expr(ctx, expr, Type::Unspecified)?; if ctx.use_agg_funcs { - self.check_projection_on_field_expr(&mut CheckContext::default(), expr)?; + let mut chk_ctx = CheckContext { + use_agg_func: ctx.use_agg_funcs, + ..Default::default() + }; + + self.check_projection_on_field_expr(&mut chk_ctx, expr)?; } else { self.reject_constant_func(&expr.attrs, app)?; } @@ -760,6 +773,11 @@ impl<'a> Analysis<'a> { Ok(tpe) } + Value::Id(_) if ctx.use_agg_funcs => Err(AnalysisError::ExpectAggExpr( + expr.attrs.pos.line, + expr.attrs.pos.col, + )), + Value::Id(id) => { if let Some(tpe) = self.scope.entries.get(id.as_str()).cloned() { Ok(tpe) @@ -772,6 +790,11 @@ impl<'a> Analysis<'a> { } } + Value::Access(_) if ctx.use_agg_funcs => Err(AnalysisError::ExpectAggExpr( + expr.attrs.pos.line, + expr.attrs.pos.col, + )), + Value::Access(access) => { let mut current = &access.target.value; diff --git a/src/tests/analysis.rs b/src/tests/analysis.rs index 6cac2e0..bf52ffe 100644 --- a/src/tests/analysis.rs +++ b/src/tests/analysis.rs @@ -235,3 +235,24 @@ fn test_analyze_accept_group_by_with_order_by_with_agg() { .unwrap(); insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); } + +#[test] +fn test_analyze_reject_group_by_no_agg() { + let query = parse_query(include_str!("./resources/reject_group_by_no_agg.eql")).unwrap(); + insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); +} + +#[test] +fn test_analyze_reject_group_by_no_agg_in_rec() { + let query = parse_query(include_str!( + "./resources/reject_group_by_no_agg_in_rec.eql" + )) + .unwrap(); + insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); +} + +#[test] +fn test_analyze_accept_group_by_with_agg_rec() { + let query = parse_query(include_str!("./resources/accept_group_by_with_agg_rec.eql")).unwrap(); + insta::assert_yaml_snapshot!(query.run_static_analysis(&Default::default())); +} diff --git a/src/tests/resources/accept_group_by_with_agg_rec.eql b/src/tests/resources/accept_group_by_with_agg_rec.eql new file mode 100644 index 0000000..4bf1a40 --- /dev/null +++ b/src/tests/resources/accept_group_by_with_agg_rec.eql @@ -0,0 +1,7 @@ + +FROM e IN events +GROUP BY e.data.department +PROJECT INTO { + department: UNIQUE(e.data.department), + cost: AVG(e.data.salary) +} diff --git a/src/tests/resources/reject_group_by_no_agg.eql b/src/tests/resources/reject_group_by_no_agg.eql new file mode 100644 index 0000000..1e4e2a4 --- /dev/null +++ b/src/tests/resources/reject_group_by_no_agg.eql @@ -0,0 +1,3 @@ +FROM e IN events +GROUP BY e.data.department +PROJECT INTO e.data.salary diff --git a/src/tests/resources/reject_group_by_no_agg_in_rec.eql b/src/tests/resources/reject_group_by_no_agg_in_rec.eql new file mode 100644 index 0000000..1544312 --- /dev/null +++ b/src/tests/resources/reject_group_by_no_agg_in_rec.eql @@ -0,0 +1,5 @@ +FROM e IN events +GROUP BY e.data.department +PROJECT INTO { + cost: e.data.salary +} diff --git a/src/tests/snapshots/eventql_parser__tests__analysis__analyze_accept_group_by_with_agg_rec.snap b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_accept_group_by_with_agg_rec.snap new file mode 100644 index 0000000..051e21c --- /dev/null +++ b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_accept_group_by_with_agg_rec.snap @@ -0,0 +1,123 @@ +--- +source: src/tests/analysis.rs +expression: "query.run_static_analysis(&Default::default())" +--- +Ok: + attrs: + pos: + line: 2 + col: 1 + sources: + - binding: + name: e + pos: + line: 2 + col: 6 + kind: + Name: events + predicate: ~ + group_by: + expr: + attrs: + pos: + line: 3 + col: 10 + value: + Access: + target: + attrs: + pos: + line: 3 + col: 10 + value: + Access: + target: + attrs: + pos: + line: 3 + col: 10 + value: + Id: e + field: data + field: department + predicate: ~ + order_by: ~ + limit: ~ + projection: + attrs: + pos: + line: 4 + col: 14 + value: + Record: + - name: department + value: + attrs: + pos: + line: 5 + col: 14 + value: + App: + func: UNIQUE + args: + - attrs: + pos: + line: 5 + col: 21 + value: + Access: + target: + attrs: + pos: + line: 5 + col: 21 + value: + Access: + target: + attrs: + pos: + line: 5 + col: 21 + value: + Id: e + field: data + field: department + - name: cost + value: + attrs: + pos: + line: 6 + col: 8 + value: + App: + func: AVG + args: + - attrs: + pos: + line: 6 + col: 12 + value: + Access: + target: + attrs: + pos: + line: 6 + col: 12 + value: + Access: + target: + attrs: + pos: + line: 6 + col: 12 + value: + Id: e + field: data + field: salary + distinct: false + meta: + project: + Record: + cost: Number + department: Unspecified + aggregate: true diff --git a/src/tests/snapshots/eventql_parser__tests__analysis__analyze_reject_group_by_no_agg.snap b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_reject_group_by_no_agg.snap new file mode 100644 index 0000000..e84962b --- /dev/null +++ b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_reject_group_by_no_agg.snap @@ -0,0 +1,9 @@ +--- +source: src/tests/analysis.rs +expression: "query.run_static_analysis(&Default::default())" +--- +Err: + Analysis: + ExpectAggExpr: + - 3 + - 14 diff --git a/src/tests/snapshots/eventql_parser__tests__analysis__analyze_reject_group_by_no_agg_in_rec.snap b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_reject_group_by_no_agg_in_rec.snap new file mode 100644 index 0000000..af1328f --- /dev/null +++ b/src/tests/snapshots/eventql_parser__tests__analysis__analyze_reject_group_by_no_agg_in_rec.snap @@ -0,0 +1,9 @@ +--- +source: src/tests/analysis.rs +expression: "query.run_static_analysis(&Default::default())" +--- +Err: + Analysis: + UnallowedAggFuncUsageWithSrcField: + - 4 + - 8