From 10b823cf6206c46a4d1c40be6b89c11528ba5718 Mon Sep 17 00:00:00 2001 From: Matthew Nibecker Date: Wed, 11 Oct 2023 14:51:31 -0700 Subject: [PATCH] make it an expr instead --- compiler/ast/dag/expr.go | 38 +------------- compiler/ast/dag/unpack.go | 2 - compiler/kernel/expr.go | 51 +++++++++++-------- compiler/kernel/groupby.go | 4 +- compiler/kernel/op.go | 4 +- compiler/optimizer/op.go | 11 ++-- compiler/optimizer/optimizer.go | 2 +- compiler/optimizer/parallelize.go | 2 +- compiler/semantic/expr.go | 83 +++++++++---------------------- compiler/semantic/op.go | 16 +++--- compiler/semantic/sql.go | 6 +-- runtime/expr/path.go | 6 +-- zfmt/dag.go | 20 +------- 13 files changed, 79 insertions(+), 166 deletions(-) diff --git a/compiler/ast/dag/expr.go b/compiler/ast/dag/expr.go index a2cabeeccb..a735d4133a 100644 --- a/compiler/ast/dag/expr.go +++ b/compiler/ast/dag/expr.go @@ -4,9 +4,6 @@ type ( Expr interface { ExprDAG() } - PathElem interface { - pathElem() - } RecordElem interface { recordAST() } @@ -30,7 +27,7 @@ type ( } Assignment struct { Kind string `json:"kind" unpack:""` - LHS Path `json:"lhs"` + LHS Expr `json:"lhs"` RHS Expr `json:"rhs"` } BinaryExpr struct { @@ -75,10 +72,6 @@ type ( Exprs []Expr `json:"exprs"` Body Seq `json:"body"` } - Path struct { - Kind string `json:"kind" unpack:""` - Path []PathElem `json:"path"` - } RecordExpr struct { Kind string `json:"kind" unpack:""` Elems []RecordElem `json:"elems"` @@ -130,7 +123,6 @@ func (*Func) ExprDAG() {} func (*Literal) ExprDAG() {} func (*MapExpr) ExprDAG() {} func (*OverExpr) ExprDAG() {} -func (*Path) ExprDAG() {} func (*RecordExpr) ExprDAG() {} func (*RegexpMatch) ExprDAG() {} func (*RegexpSearch) ExprDAG() {} @@ -167,34 +159,6 @@ func (*Spread) recordAST() {} func (*Spread) vectorElem() {} func (*VectorValue) vectorElem() {} -func (p *Path) StaticPath() *This { - this := &This{Kind: "This"} - for _, elem := range p.Path { - p, ok := elem.(*StaticPathElem) - if !ok { - return nil - } - this.Path = append(this.Path, p.Name) - } - return this -} - -func NewStaticPath(path ...string) *Path { - p := Path{Kind: "Path"} - for _, name := range path { - p.Path = append(p.Path, &StaticPathElem{Kind: "StaticPathElem", Name: name}) - } - return &p -} - -type StaticPathElem struct { - Kind string `json:"kind" unpack:""` - Name string `json:"name"` -} - -func (*This) pathElem() {} -func (*StaticPathElem) pathElem() {} - func NewBinaryExpr(op string, lhs, rhs Expr) *BinaryExpr { return &BinaryExpr{ Kind: "BinaryExpr", diff --git a/compiler/ast/dag/unpack.go b/compiler/ast/dag/unpack.go index 0bb9b5a85e..0a03034870 100644 --- a/compiler/ast/dag/unpack.go +++ b/compiler/ast/dag/unpack.go @@ -36,7 +36,6 @@ var unpacker = unpack.New( Over{}, OverExpr{}, Pass{}, - Path{}, PoolScan{}, Put{}, RecordExpr{}, @@ -52,7 +51,6 @@ var unpacker = unpack.New( Slicer{}, Sort{}, Spread{}, - StaticPathElem{}, Summarize{}, Switch{}, Tail{}, diff --git a/compiler/kernel/expr.go b/compiler/kernel/expr.go index 9113ff570d..0a9d837e85 100644 --- a/compiler/kernel/expr.go +++ b/compiler/kernel/expr.go @@ -65,12 +65,6 @@ func (b *Builder) compileExpr(e dag.Expr) (expr.Evaluator, error) { return expr.NewDottedExpr(b.zctx(), field.Path(e.Path)), nil case *dag.Dot: return b.compileDotExpr(e) - case *dag.Path: - // Path only works as a general expression if it is a static path. - if this := e.StaticPath(); this != nil { - return expr.NewDottedExpr(b.zctx(), field.Path(this.Path)), nil - } - return nil, fmt.Errorf("internal error: invalid path expression %s", e) case *dag.UnaryExpr: return b.compileUnary(*e) case *dag.BinaryExpr: @@ -269,23 +263,38 @@ func (b *Builder) compileDotExpr(dot *dag.Dot) (expr.Evaluator, error) { return expr.NewDotExpr(b.zctx(), record, dot.RHS), nil } -func (b *Builder) compilePath(e dag.Path) (*expr.Path, error) { - elems := make([]expr.PathElem, 0, len(e.Path)) - for _, elem := range e.Path { - switch e := elem.(type) { - case *dag.This: - eval, err := b.compileExpr(e) - if err != nil { - return nil, err - } - elems = append(elems, expr.NewPathElemExpr(b.octx.Zctx, eval)) - case *dag.StaticPathElem: - elems = append(elems, &expr.StaticPathElem{Name: e.Name}) - default: - return nil, fmt.Errorf("internal error: invalid lval type %T", e) +func (b *Builder) compilePath(e dag.Expr) (*expr.Path, error) { + switch e := e.(type) { + case *dag.This: + var elems []expr.PathElem + for _, elem := range e.Path { + elems = append(elems, &expr.StaticPathElem{Name: elem}) + } + return expr.NewPath(elems), nil + case *dag.BinaryExpr: + if e.Op != "[" { + return nil, fmt.Errorf("internal error: invalid path expression %s", e) } + lhs, err := b.compilePath(e.LHS) + if err != nil { + return nil, err + } + rhs, err := b.compileExpr(e.RHS) + if err != nil { + return nil, err + } + lhs.Elems = append(lhs.Elems, expr.NewPathElemExpr(b.zctx(), rhs)) + return lhs, nil + case *dag.Dot: + lhs, err := b.compilePath(e.LHS) + if err != nil { + return nil, err + } + lhs.Elems = append(lhs.Elems, &expr.StaticPathElem{Name: e.RHS}) + return lhs, nil + default: + return nil, fmt.Errorf("internal error: invalid path expression %s", e) } - return expr.NewPath(elems), nil } func (b *Builder) compileAssignment(node *dag.Assignment) (expr.Assignment, error) { diff --git a/compiler/kernel/groupby.go b/compiler/kernel/groupby.go index 300f86a46d..7c0762995d 100644 --- a/compiler/kernel/groupby.go +++ b/compiler/kernel/groupby.go @@ -44,8 +44,8 @@ func (b *Builder) compileAggAssignment(assignment dag.Assignment) (field.Path, * if !ok { return nil, nil, errors.New("aggregator is not an aggregation expression") } - this := assignment.LHS.StaticPath() - if this == nil { + this, ok := assignment.LHS.(*dag.This) + if !ok { return nil, nil, errors.New("internal error: aggregator assignment must be a static path") } m, err := b.compileAgg(aggAST) diff --git a/compiler/kernel/op.go b/compiler/kernel/op.go index 7aa22d5e58..d40fdc56e9 100644 --- a/compiler/kernel/op.go +++ b/compiler/kernel/op.go @@ -357,8 +357,8 @@ func (b *Builder) compileStaticAssignments(assignments []dag.Assignment) ([]fiel lhs := make([]field.Path, 0, len(assignments)) rhs := make([]expr.Evaluator, 0, len(assignments)) for _, a := range assignments { - this := a.LHS.StaticPath() - if this == nil { + this, ok := a.LHS.(*dag.This) + if !ok { return nil, nil, errors.New("internal error: dynamic path in assignment when expecting a static path") } lhs = append(lhs, slices.Clone(this.Path)) diff --git a/compiler/optimizer/op.go b/compiler/optimizer/op.go index 926f530e10..c1d793b575 100644 --- a/compiler/optimizer/op.go +++ b/compiler/optimizer/op.go @@ -66,7 +66,7 @@ func (o *Optimizer) analyzeSortKey(op dag.Op, in order.SortKey) (order.SortKey, return order.Nil, nil case *dag.Put: for _, assignment := range op.Args { - if fieldOf(&assignment.LHS).Equal(key) { + if fieldOf(assignment.LHS).Equal(key) { return order.Nil, nil } } @@ -101,7 +101,7 @@ func isKeyOfSummarize(summarize *dag.Summarize, in order.SortKey) bool { } key := in.Keys[0] for _, outputKeyExpr := range summarize.Keys { - groupByKey := fieldOf(&outputKeyExpr.LHS) + groupByKey := fieldOf(outputKeyExpr.LHS) if groupByKey.Equal(key) { rhsExpr := outputKeyExpr.RHS rhs := fieldOf(rhsExpr) @@ -147,7 +147,7 @@ func analyzeCuts(assignments []dag.Assignment, sortKey order.SortKey) order.Sort scoreboard := make(map[string]field.Path) scoreboard[fieldKey(key)] = key for _, a := range assignments { - lhs := fieldOf(&a.LHS) + lhs := fieldOf(a.LHS) rhs := fieldOf(a.RHS) if lhs == nil { // If we cannot statically determine the data flow, @@ -211,11 +211,6 @@ func fieldOf(e dag.Expr) field.Path { if this, ok := e.(*dag.This); ok { return this.Path } - if path, ok := e.(*dag.Path); ok { - if this := path.StaticPath(); this != nil { - return this.Path - } - } return nil } diff --git a/compiler/optimizer/optimizer.go b/compiler/optimizer/optimizer.go index ce11f1d22a..24f8f13d4c 100644 --- a/compiler/optimizer/optimizer.go +++ b/compiler/optimizer/optimizer.go @@ -312,7 +312,7 @@ func (o *Optimizer) propagateSortKeyOp(op dag.Op, parents []order.SortKey) ([]or //XXX handle only primary key for now key := parent.Primary() for _, k := range op.Keys { - if groupByKey := fieldOf(&k.LHS); groupByKey.Equal(key) { + if groupByKey := fieldOf(k.LHS); groupByKey.Equal(key) { rhsExpr := k.RHS rhs := fieldOf(rhsExpr) if rhs.Equal(key) || orderPreservingCall(rhsExpr, groupByKey) { diff --git a/compiler/optimizer/parallelize.go b/compiler/optimizer/parallelize.go index 9136eb7ed3..b1f034fc30 100644 --- a/compiler/optimizer/parallelize.go +++ b/compiler/optimizer/parallelize.go @@ -134,7 +134,7 @@ func (o *Optimizer) liftIntoParPaths(ops []dag.Op) { // so the ingress aggregator should simply reference the key // by its name. This loop updates the ingress to do so. for k := range op.Keys { - op.Keys[k].RHS = &op.Keys[k].LHS + op.Keys[k].RHS = op.Keys[k].LHS } case *dag.Sort: if len(op.Args) != 1 { diff --git a/compiler/semantic/expr.go b/compiler/semantic/expr.go index 069059c3a4..d2b316d976 100644 --- a/compiler/semantic/expr.go +++ b/compiler/semantic/expr.go @@ -420,55 +420,6 @@ func (a *analyzer) semBinary(e *ast.BinaryExpr) (dag.Expr, error) { }, nil } -func (a *analyzer) semPath(e ast.Expr) (*dag.Path, error) { - switch e := e.(type) { - case *ast.BinaryExpr: - if e.Op == "." { - lhs, err := a.semPath(e.LHS) - if err != nil || lhs == nil { - return nil, err - } - id, ok := e.RHS.(*ast.ID) - if !ok { - return nil, nil - } - lhs.Path = append(lhs.Path, &dag.StaticPathElem{Kind: "StaticPathElem", Name: id.Name}) - return lhs, nil - } - if e.Op == "[" { - lhs, err := a.semPath(e.LHS) - if lhs == nil || err != nil { - return nil, nil - } - rhs, err := a.semExpr(e.RHS) - if err != nil { - return nil, err - } - if this, ok := rhs.(*dag.This); ok { - lhs.Path = append(lhs.Path, this) - return lhs, nil - } - if p, ok := isStringConst(a.zctx, rhs); ok { - lhs.Path = append(lhs.Path, &dag.StaticPathElem{Kind: "StaticPathElem", Name: p}) - return lhs, nil - } - } - return nil, nil - case *ast.ID: - id, err := a.semID(e) - if err != nil { - return nil, err - } - if this, ok := id.(*dag.This); ok { - return dag.NewStaticPath(this.Path...), nil - } - return nil, nil - } - // This includes a null Expr, which can happen if the AST is missing - // a field or sets it to null. - return nil, nil -} - func (a *analyzer) isIndexOfThis(lhs, rhs dag.Expr) *dag.This { if this, ok := lhs.(*dag.This); ok { if s, ok := isStringConst(a.zctx, rhs); ok { @@ -573,30 +524,44 @@ func (a *analyzer) semAssignment(e ast.Assignment) (dag.Assignment, error) { if err != nil { return dag.Assignment{}, err } - var lhs *dag.Path + var lhs dag.Expr if e.LHS == nil { path, err := derriveLHSPath(rhs) if err != nil { return dag.Assignment{}, err } - lhs = dag.NewStaticPath(path...) + lhs = &dag.This{Kind: "This", Path: path} } else { - if lhs, err = a.semPath(e.LHS); lhs == nil { - if err == nil { - err = errors.New("illegal left-hand side of assignment") - } + if lhs, err = a.semExpr(e.LHS); err != nil { return dag.Assignment{}, err } } - if len(lhs.Path) == 0 { - return dag.Assignment{}, errors.New("cannot assign to 'this'") + if !a.isValidPath(lhs) { + return dag.Assignment{}, errors.New("illegal left-hand side of assignment") + } + if this, ok := lhs.(*dag.This); ok { + if len(this.Path) == 0 { + return dag.Assignment{}, errors.New("cannot assign to 'this'") + } + } + return dag.Assignment{Kind: "Assignment", LHS: lhs, RHS: rhs}, nil +} + +func (a *analyzer) isValidPath(e dag.Expr) bool { + switch e := e.(type) { + case *dag.This: + return true + case *dag.Dot: + return a.isValidPath(e.LHS) + case *dag.BinaryExpr: + return e.Op == "[" && a.isValidPath(e.LHS) } - return dag.Assignment{Kind: "Assignment", LHS: *lhs, RHS: rhs}, nil + return false } func assignHasDynamicPath(assignments []dag.Assignment) bool { for _, a := range assignments { - if a.LHS.StaticPath() == nil { + if _, ok := a.LHS.(*dag.This); !ok { return true } } diff --git a/compiler/semantic/op.go b/compiler/semantic/op.go index 88c3f37c08..510305092f 100644 --- a/compiler/semantic/op.go +++ b/compiler/semantic/op.go @@ -510,7 +510,7 @@ func (a *analyzer) semOp(o ast.Op, seq dag.Seq) (dag.Seq, error) { // Collect static paths so we can check on what is available. var fields field.List for _, a := range assignments { - if this := a.LHS.StaticPath(); this != nil { + if this, ok := a.LHS.(*dag.This); ok { fields = append(fields, this.Path) } } @@ -619,7 +619,7 @@ func (a *analyzer) semOp(o ast.Op, seq dag.Seq) (dag.Seq, error) { // We can do collision checking on static paths, so check what we can. var fields field.List for _, a := range assignments { - if this := a.LHS.StaticPath(); this != nil { + if this, ok := a.LHS.(*dag.This); ok { fields = append(fields, this.Path) } } @@ -804,14 +804,14 @@ func (a *analyzer) semOp(o ast.Op, seq dag.Seq) (dag.Seq, error) { Aggs: []dag.Assignment{ { Kind: "Assignment", - LHS: *dag.NewStaticPath("sample"), + LHS: pathOf("sample"), RHS: &dag.Agg{Kind: "Agg", Name: "any", Expr: e}, }, }, Keys: []dag.Assignment{ { Kind: "Assignment", - LHS: *dag.NewStaticPath("shape"), + LHS: pathOf("shape"), RHS: &dag.Call{Kind: "Call", Name: "typeof", Args: []dag.Expr{e}}, }, }, @@ -860,8 +860,8 @@ func (a *analyzer) singletonAgg(agg ast.Assignment, seq dag.Seq) dag.Seq { yield := &dag.Yield{ Kind: "Yield", } - this := out.LHS.StaticPath() - if this == nil || len(this.Path) != 1 { + this, ok := out.LHS.(*dag.This) + if !ok || len(this.Path) != 1 { return nil } yield.Exprs = append(yield.Exprs, this) @@ -988,7 +988,7 @@ func (a *analyzer) semOpAssignment(p *ast.OpAssignment) (dag.Op, error) { return nil, err } if _, ok := a.RHS.(*dag.Agg); ok { - if a.LHS.StaticPath() == nil { + if _, ok := a.LHS.(*dag.This); !ok { return nil, errors.New("summarize: illegal use of dynamic assignment in aggregation") } aggs = append(aggs, a) @@ -1075,7 +1075,7 @@ func (a *analyzer) semCallOp(call *ast.Call, seq dag.Seq) (dag.Seq, error) { Aggs: []dag.Assignment{ { Kind: "Assignment", - LHS: *dag.NewStaticPath(call.Name), + LHS: pathOf(call.Name), RHS: agg, }, }, diff --git a/compiler/semantic/sql.go b/compiler/semantic/sql.go index 2bdd6a8047..72e2f52319 100644 --- a/compiler/semantic/sql.go +++ b/compiler/semantic/sql.go @@ -238,7 +238,7 @@ func (a *analyzer) convertSQLAlias(e ast.Expr) (*dag.Cut, string, error) { } assignment := dag.Assignment{ Kind: "Assignment", - LHS: *dag.NewStaticPath(fld.Path...), + LHS: fld, RHS: &dag.This{Kind: "This"}, } return &dag.Cut{ @@ -291,7 +291,7 @@ func (a *analyzer) convertSQLJoin(leftPath []dag.Op, sqlJoin ast.SQLJoin) ([]dag } alias := dag.Assignment{ Kind: "Assignment", - LHS: *dag.NewStaticPath(aliasID), + LHS: pathOf(aliasID), RHS: &dag.This{Kind: "This", Path: field.Path{aliasID}}, } join := &dag.Join{ @@ -519,7 +519,7 @@ func (a *analyzer) deriveAs(as ast.Assignment) (field.Path, error) { if err != nil { return nil, fmt.Errorf("AS clause of SELECT: %w", err) } - if this := sa.LHS.StaticPath(); this != nil { + if this, ok := sa.LHS.(*dag.This); ok { return this.Path, nil } return nil, fmt.Errorf("AS clause not a field: %w", err) diff --git a/runtime/expr/path.go b/runtime/expr/path.go index 6b1f0f0af9..d8d0e7f8c4 100644 --- a/runtime/expr/path.go +++ b/runtime/expr/path.go @@ -10,19 +10,19 @@ type PathElem interface { } type Path struct { - elems []PathElem + Elems []PathElem cache field.Path } func NewPath(evals []PathElem) *Path { - return &Path{elems: evals} + return &Path{Elems: evals} } // Eval returns the path of the lval. If there's an error the returned *zed.Value // will not be nill. func (l *Path) Eval(ectx Context, this *zed.Value) (field.Path, *zed.Value) { l.cache = l.cache[:0] - for _, e := range l.elems { + for _, e := range l.Elems { name, val := e.Eval(ectx, this) if val != nil { return nil, val diff --git a/zfmt/dag.go b/zfmt/dag.go index 5fb5be6c3e..a334cc85ab 100644 --- a/zfmt/dag.go +++ b/zfmt/dag.go @@ -49,7 +49,7 @@ func (c *canonDAG) assignments(assignments []dag.Assignment) { if k > 0 { c.write(",") } - c.expr(&a.LHS, "") + c.expr(a.LHS, "") c.write(":=") c.expr(a.RHS, "") } @@ -119,24 +119,6 @@ func (c *canonDAG) expr(e dag.Expr, parent string) { c.write("search(%s)", e.Value) case *dag.This: c.fieldpath(e.Path) - case *dag.Path: - if p := e.StaticPath(); p != nil { - c.fieldpath(p.Path) - return - } - for k, elem := range e.Path { - if k == 0 { - c.write("this") - } - c.write("[") - switch elem := elem.(type) { - case *dag.This: - c.fieldpath(elem.Path) - case *dag.StaticPathElem: - c.write(zson.QuotedName(elem.Name)) - } - c.write("]") - } case *dag.Var: c.write("%s", e.Name) case *dag.Literal: