From 60fa04208188306d0db68f2e83ccef1397768dc9 Mon Sep 17 00:00:00 2001 From: Matthew Nibecker Date: Tue, 17 Oct 2023 10:31:04 -0700 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Noah Treuhaft --- compiler/kernel/groupby.go | 2 +- compiler/semantic/op.go | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/compiler/kernel/groupby.go b/compiler/kernel/groupby.go index 1887b9b1a6..eb92374add 100644 --- a/compiler/kernel/groupby.go +++ b/compiler/kernel/groupby.go @@ -45,7 +45,7 @@ func (b *Builder) compileAggAssignment(assignment dag.Assignment) (field.Path, * } this, ok := assignment.LHS.(*dag.This) if !ok { - return nil, nil, errors.New("internal error: aggregator assignment LHS must be a static path") + return nil, nil, errors.New("internal error: aggregator assignment LHS is not a static path: %#v", assignment.LHS) } m, err := b.compileAgg(aggAST) return this.Path, m, err diff --git a/compiler/semantic/op.go b/compiler/semantic/op.go index fdc55bb206..ebcd3d4485 100644 --- a/compiler/semantic/op.go +++ b/compiler/semantic/op.go @@ -416,7 +416,7 @@ func (a *analyzer) semOp(o ast.Op, seq dag.Seq) (dag.Seq, error) { return nil, err } if assignmentHasDynamicLHS(keys) { - return nil, errors.New("summarize: keys must be static field references") + return nil, errors.New("summarize: key output fields must be static") } if len(keys) == 0 && len(o.Aggs) == 1 { if seq := a.singletonAgg(o.Aggs[0], seq); seq != nil { @@ -428,7 +428,7 @@ func (a *analyzer) semOp(o ast.Op, seq dag.Seq) (dag.Seq, error) { return nil, err } if assignmentHasDynamicLHS(aggs) { - return nil, errors.New("summarize: aggs must be static field references") + return nil, errors.New("summarize: aggregate output field must be static") } // Note: InputSortDir is copied in here but it's not meaningful // coming from a parser AST, only from a worker using the kernel DSL, @@ -967,8 +967,6 @@ func (a *analyzer) semOpAssignment(p *ast.OpAssignment) (dag.Op, error) { var aggs, puts []dag.Assignment for _, assign := range p.Assignments { // Parition assignments into agg vs. puts. - // It's okay to pass false here for the summarize bool because - // semAssignment will check if the RHS is a dag.Agg and override. a, err := a.semAssignment(assign) if err != nil { return nil, err