diff --git a/compiler/kernel/op.go b/compiler/kernel/op.go index ba81ef9969..3e9a613e8e 100644 --- a/compiler/kernel/op.go +++ b/compiler/kernel/op.go @@ -174,10 +174,18 @@ func (b *Builder) compileLeaf(o dag.Op, parent zbuf.Puller) (zbuf.Puller, error) putter := expr.NewPutter(b.octx.Zctx, clauses) return op.NewApplier(b.octx, parent, putter), nil case *dag.Rename: - var srcs, dsts field.List + var srcs, dsts []*expr.Lval for _, a := range v.Args { - dsts = append(dsts, a.LHS.(*dag.This).Path) - srcs = append(srcs, a.RHS.(*dag.This).Path) + src, err := b.compileLval(a.RHS) + if err != nil { + return nil, err + } + dst, err := b.compileLval(a.LHS) + if err != nil { + return nil, err + } + srcs = append(srcs, src) + dsts = append(dsts, dst) } renamer := expr.NewRenamer(b.octx.Zctx, srcs, dsts) return op.NewApplier(b.octx, parent, renamer), nil diff --git a/compiler/semantic/op.go b/compiler/semantic/op.go index cc20581d92..aa9d6a1bba 100644 --- a/compiler/semantic/op.go +++ b/compiler/semantic/op.go @@ -640,25 +640,23 @@ func (a *analyzer) semOp(o ast.Op, seq dag.Seq) (dag.Seq, error) { case *ast.Rename: var assignments []dag.Assignment for _, fa := range o.Args { - dst, err := a.semField(fa.LHS) + assign, err := a.semAssignment(fa) if err != nil { - return nil, errors.New("rename: requires explicit field references") + return nil, fmt.Errorf("rename: %w", err) } - src, err := a.semField(fa.RHS) - if err != nil { - return nil, errors.New("rename: requires explicit field references") - } - if len(dst.Path) != len(src.Path) { - return nil, fmt.Errorf("rename: cannot rename %s to %s", src, dst) + if !isLval(assign.RHS) { + return nil, fmt.Errorf("rename: illegal right-hand side of assignment") } - // Check that the prefixes match and, if not, report first place - // that they don't. - for i := 0; i <= len(src.Path)-2; i++ { - if src.Path[i] != dst.Path[i] { - return nil, fmt.Errorf("rename: cannot rename %s to %s (differ in %s vs %s)", src, dst, src.Path[i], dst.Path[i]) + // If both paths are static validate them. Otherwise this will be + // done at runtime. + lhs, lhsOk := assign.LHS.(*dag.This) + rhs, rhsOk := assign.RHS.(*dag.This) + if rhsOk && lhsOk { + if err := expr.CheckRenameField(lhs.Path, rhs.Path); err != nil { + return nil, fmt.Errorf("rename: %w", err) } } - assignments = append(assignments, dag.Assignment{Kind: "Assignment", LHS: dst, RHS: src}) + assignments = append(assignments, assign) } return append(seq, &dag.Rename{ Kind: "Rename", diff --git a/docs/language/operators/rename.md b/docs/language/operators/rename.md index 70be76e194..9003dd12d7 100644 --- a/docs/language/operators/rename.md +++ b/docs/language/operators/rename.md @@ -49,7 +49,7 @@ echo '{a:1,r:{b:2,c:3}}' | zq -z 'rename w:=r.b' - ``` => ```mdtest-output -rename: cannot rename r.b to w +rename: left-hand side and right-hand side must have the same depth (r.b vs w) ``` _Record literals can be used instead of rename for mutation_ ```mdtest-command diff --git a/docs/tutorials/schools.md b/docs/tutorials/schools.md index 790e3edde8..68debefa3b 100644 --- a/docs/tutorials/schools.md +++ b/docs/tutorials/schools.md @@ -843,7 +843,7 @@ zq -Z 'rename toplevel:=outer.inner' nested.zson ``` produces this compile-time error message and the query is not run: ```mdtest-output -rename: cannot rename outer.inner to toplevel +rename: left-hand side and right-hand side must have the same depth (outer.inner vs toplevel) ``` This goal could instead be achieved by combining [`put`](#44-put) and [`drop`](#42-drop), e.g., diff --git a/pkg/field/field.go b/pkg/field/field.go index 2a6a2e567f..6a2e158fa0 100644 --- a/pkg/field/field.go +++ b/pkg/field/field.go @@ -8,11 +8,20 @@ import ( type Path []string -func (p Path) String() string { +func (p Path) String() string { return string(p.AppendTo(nil)) } + +// AppendTo appends the string representation of the path to byte slice b. +func (p Path) AppendTo(b []byte) []byte { if len(p) == 0 { - return "this" + return append(b, "this"...) + } + for i, s := range p { + if i > 0 { + b = append(b, '.') + } + b = append(b, s...) } - return strings.Join(p, ".") + return b } func (p Path) Leaf() string { @@ -57,12 +66,17 @@ func DottedList(s string) List { type List []Path -func (l List) String() string { - paths := make([]string, 0, len(l)) - for _, f := range l { - paths = append(paths, f.String()) +func (l List) String() string { return string(l.AppendTo(nil)) } + +// AppendTo appends the string representation of the list to byte slice b. +func (l List) AppendTo(b []byte) []byte { + for i, p := range l { + if i > 0 { + b = append(b, ',') + } + b = p.AppendTo(b) } - return strings.Join(paths, ",") + return b } func (l List) Has(in Path) bool { diff --git a/runtime/expr/renamer.go b/runtime/expr/renamer.go index 2735f0a751..06b5e499cb 100644 --- a/runtime/expr/renamer.go +++ b/runtime/expr/renamer.go @@ -17,13 +17,85 @@ type Renamer struct { zctx *zed.Context // For the dst field name, we just store the leaf name since the // src path and the dst path are the same and only differ in the leaf name. - srcs field.List - dsts field.List - typeMap map[int]*zed.TypeRecord + srcs []*Lval + dsts []*Lval + typeMap map[int]map[string]*zed.TypeRecord + // fieldsStr is used to reduce allocations when computing the fields id. + fieldsStr []byte } -func NewRenamer(zctx *zed.Context, srcs, dsts field.List) *Renamer { - return &Renamer{zctx, srcs, dsts, make(map[int]*zed.TypeRecord)} +func NewRenamer(zctx *zed.Context, srcs, dsts []*Lval) *Renamer { + return &Renamer{zctx, srcs, dsts, make(map[int]map[string]*zed.TypeRecord), nil} +} + +func (r *Renamer) Eval(ectx Context, this *zed.Value) *zed.Value { + if !zed.IsRecordType(this.Type) { + return this + } + srcs, dsts, err := r.evalFields(ectx, this) + if err != nil { + return ectx.CopyValue(*r.zctx.WrapError(fmt.Sprintf("rename: %s", err), this)) + } + id := this.Type.ID() + m, ok := r.typeMap[id] + if !ok { + m = make(map[string]*zed.TypeRecord) + r.typeMap[id] = m + } + r.fieldsStr = dsts.AppendTo(srcs.AppendTo(r.fieldsStr[:0])) + typ, ok := m[string(r.fieldsStr)] + if !ok { + var err error + typ, err = r.computeType(zed.TypeRecordOf(this.Type), srcs, dsts) + if err != nil { + return ectx.CopyValue(*r.zctx.WrapError(fmt.Sprintf("rename: %s", err), this)) + } + m[string(r.fieldsStr)] = typ + } + return ectx.NewValue(typ, this.Bytes()) +} + +func CheckRenameField(src, dst field.Path) error { + if len(src) != len(dst) { + return fmt.Errorf("left-hand side and right-hand side must have the same depth (%s vs %s)", src, dst) + } + for i := 0; i <= len(src)-2; i++ { + if src[i] != dst[i] { + return fmt.Errorf("cannot rename %s to %s (differ in %s vs %s)", src, dst, src[i], dst[i]) + } + } + return nil +} + +func (r *Renamer) evalFields(ectx Context, this *zed.Value) (field.List, field.List, error) { + var srcs, dsts field.List + for i := range r.srcs { + src, err := r.srcs[i].Eval(ectx, this) + if err != nil { + return nil, nil, err + } + dst, err := r.dsts[i].Eval(ectx, this) + if err != nil { + return nil, nil, err + } + if err := CheckRenameField(src, dst); err != nil { + return nil, nil, err + } + srcs = append(srcs, src) + dsts = append(dsts, dst) + } + return srcs, dsts, nil +} + +func (r *Renamer) computeType(typ *zed.TypeRecord, srcs, dsts field.List) (*zed.TypeRecord, error) { + for k, dst := range dsts { + var err error + typ, err = r.dstType(typ, srcs[k], dst) + if err != nil { + return nil, err + } + } + return typ, nil } func (r *Renamer) dstType(typ *zed.TypeRecord, src, dst field.Path) (*zed.TypeRecord, error) { @@ -57,32 +129,3 @@ func (r *Renamer) dstType(typ *zed.TypeRecord, src, dst field.Path) (*zed.TypeRe } return typ, nil } - -func (r *Renamer) computeType(typ *zed.TypeRecord) (*zed.TypeRecord, error) { - for k, dst := range r.dsts { - var err error - typ, err = r.dstType(typ, r.srcs[k], dst) - if err != nil { - return nil, err - } - } - return typ, nil -} - -func (r *Renamer) Eval(ectx Context, this *zed.Value) *zed.Value { - if !zed.IsRecordType(this.Type) { - return this - } - id := this.Type.ID() - typ, ok := r.typeMap[id] - if !ok { - var err error - typ, err = r.computeType(zed.TypeRecordOf(this.Type)) - if err != nil { - return r.zctx.WrapError(fmt.Sprintf("rename: %s", err), this) - } - r.typeMap[id] = typ - } - out := this.Copy() - return ectx.NewValue(typ, out.Bytes()) -} diff --git a/runtime/expr/ztests/rename-error-move.yaml b/runtime/expr/ztests/rename-error-move.yaml index 9fc181586e..4aa70ad092 100644 --- a/runtime/expr/ztests/rename-error-move.yaml +++ b/runtime/expr/ztests/rename-error-move.yaml @@ -3,4 +3,4 @@ zed: rename dst:=id.resp_h input: | {id:{orig_h:10.164.94.120,orig_p:39681(port=uint16),resp_h:10.47.3.155,resp_p:3389(port)}} -errorRE: "rename: cannot rename id.resp_h to dst" +errorRE: "rename: left-hand side and right-hand side must have the same depth \\(dst vs id.resp_h\\)" diff --git a/runtime/op/ztests/rename-dynamic-field.yaml b/runtime/op/ztests/rename-dynamic-field.yaml new file mode 100644 index 0000000000..dab3e7f4d3 --- /dev/null +++ b/runtime/op/ztests/rename-dynamic-field.yaml @@ -0,0 +1,22 @@ +script: | + echo '{target:"foo",src:"bar"} {target:"fool",src:"baz"}' | zq -z 'rename this[target] := src' - + echo '// ===' + echo '{target:"a",a:"bar"} {target:"b",b:"baz"}' | zq -z 'rename dst := this[target]' - + # runtime error cases + echo '// ===' + echo '{foo:"a",bar:"b"}' | zq -z 'rename this[foo]["c"] := this[bar]["d"]' - + echo '// ===' + echo '{foo:"a"}' | zq -z 'rename this[foo]["c"] := this[foo]["a"]["b"]' - + +outputs: + - name: stdout + data: | + {target:"foo",foo:"bar"} + {target:"fool",fool:"baz"} + // === + {target:"a",dst:"bar"} + {target:"b",dst:"baz"} + // === + error({message:"rename: cannot rename b.d to a.c (differ in b vs a)",on:{foo:"a",bar:"b"}}) + // === + error({message:"rename: left-hand side and right-hand side must have the same depth (a.a.b vs a.c)",on:{foo:"a"}})