diff --git a/query/compile.go b/query/compile.go index d592476ec5..1c0a88dad6 100644 --- a/query/compile.go +++ b/query/compile.go @@ -16,9 +16,7 @@ import ( ) const ( - TableParameter = "table" - - tableIDKey = "id" + TableParameter = "table" tableKindKey = "kind" tableParentsKey = "parents" //tableSpecKey = "spec" @@ -51,8 +49,7 @@ func Compile(ctx context.Context, q string, opts ...Option) (*Spec, error) { s, _ = opentracing.StartSpanFromContext(ctx, "compile") defer s.Finish() - qd := new(queryDomain) - scope, decls := builtIns(qd) + scope, decls := builtIns() interpScope := interpreter.NewScopeWithValues(scope) // Convert AST program to a semantic program @@ -61,10 +58,11 @@ func Compile(ctx context.Context, q string, opts ...Option) (*Spec, error) { return nil, err } - if _, err := interpreter.Eval(semProg, interpScope); err != nil { + operations, err := interpreter.Eval(semProg, interpScope) + if err != nil { return nil, err } - spec := qd.ToSpec() + spec := toSpec(operations) if o.verbose { log.Println("Query Spec: ", Formatted(spec, FmtJSON)) @@ -72,6 +70,34 @@ func Compile(ctx context.Context, q string, opts ...Option) (*Spec, error) { return spec, nil } +func toSpec(stmtVals []values.Value) *Spec { + ider := &ider{ + id: 0, + lookup: make(map[*TableObject]OperationID), + } + + spec := new(Spec) + visited := make(map[*TableObject]bool) + nodes := make([]*TableObject, 0, len(stmtVals)) + + for _, val := range stmtVals { + if op, ok := val.(*TableObject); ok { + dup := false + for _, node := range nodes { + if op.Equal(node) { + dup = true + break + } + } + if !dup { + op.buildSpec(ider, spec, visited) + nodes = append(nodes, op) + } + } + } + return spec +} + type CreateOperationSpec func(args Arguments, a *Administration) (OperationSpec, error) var builtinScope = make(map[string]values.Value) @@ -137,7 +163,6 @@ func FinalizeBuiltIns() { } var TableObjectType = semantic.NewObjectType(map[string]semantic.Type{ - tableIDKey: semantic.String, tableKindKey: semantic.String, // TODO(nathanielc): The spec types vary significantly making type comparisons impossible, for now the solution is to state the type as an empty object. //tableSpecKey: semantic.EmptyObject, @@ -145,85 +170,144 @@ var TableObjectType = semantic.NewObjectType(map[string]semantic.Type{ tableParentsKey: semantic.NewArrayType(semantic.EmptyObject), }) +// IDer produces the mapping of table Objects to OpertionIDs +type IDer interface { + ID(*TableObject) OperationID +} + +// IDerOpSpec is the interface any operation spec that needs +// access to OperationIDs in the query spec must implement. +type IDerOpSpec interface { + IDer(ider IDer) +} + type TableObject struct { - ID OperationID + // TODO(Josh): Remove args once the + // OperationSpec interface has an Equal method. + args Arguments Kind OperationKind Spec OperationSpec Parents values.Array } -func (t TableObject) Operation() *Operation { +func (t *TableObject) Operation(ider IDer) *Operation { + if iderOpSpec, ok := t.Spec.(IDerOpSpec); ok { + iderOpSpec.IDer(ider) + } + return &Operation{ - ID: t.ID, + ID: ider.ID(t), Spec: t.Spec, } } -func (t TableObject) String() string { - return fmt.Sprintf("{id: %q, kind: %q}", t.ID, t.Kind) +type ider struct { + id int + lookup map[*TableObject]OperationID } -func (t TableObject) ToSpec() *Spec { - visited := make(map[OperationID]bool) +func (i *ider) nextID() int { + next := i.id + i.id++ + return next +} + +func (i *ider) get(t *TableObject) (OperationID, bool) { + tableID, ok := i.lookup[t] + return tableID, ok +} + +func (i *ider) set(t *TableObject, id int) OperationID { + opID := OperationID(fmt.Sprintf("%s%d", t.Kind, id)) + i.lookup[t] = opID + return opID +} + +func (i *ider) ID(t *TableObject) OperationID { + tableID, ok := i.get(t) + if !ok { + tableID = i.set(t, i.nextID()) + } + return tableID +} + +func (t *TableObject) ToSpec() *Spec { + ider := &ider{ + id: 0, + lookup: make(map[*TableObject]OperationID), + } spec := new(Spec) - t.buildSpec(spec, visited) + visited := make(map[*TableObject]bool) + t.buildSpec(ider, spec, visited) return spec } -func (t TableObject) buildSpec(spec *Spec, visited map[OperationID]bool) { - id := t.ID +func (t *TableObject) buildSpec(ider IDer, spec *Spec, visited map[*TableObject]bool) { + // Traverse graph upwards to first unvisited node t.Parents.Range(func(i int, v values.Value) { - p := v.(TableObject) - if !visited[p.ID] { + p := v.(*TableObject) + if !visited[p] { // rescurse up parents - p.buildSpec(spec, visited) + p.buildSpec(ider, spec, visited) } + }) + // Make sure parents are placed in consistent order + t.Parents.Sort(func(i, j values.Value) bool { + return ider.ID(i.(*TableObject)) < ider.ID(j.(*TableObject)) + }) + + // Assign ID to table object after visiting all ancestors. + tableID := ider.ID(t) + + // Link table object to all parents after assigning ID. + t.Parents.Range(func(i int, v values.Value) { + p := v.(*TableObject) spec.Edges = append(spec.Edges, Edge{ - Parent: p.ID, - Child: id, + Parent: ider.ID(p), + Child: tableID, }) }) - visited[id] = true - spec.Operations = append(spec.Operations, t.Operation()) + visited[t] = true + spec.Operations = append(spec.Operations, t.Operation(ider)) } -func (t TableObject) Type() semantic.Type { +func (t *TableObject) Type() semantic.Type { return TableObjectType } -func (t TableObject) Str() string { +func (t *TableObject) Str() string { panic(values.UnexpectedKind(semantic.Object, semantic.String)) } -func (t TableObject) Int() int64 { +func (t *TableObject) Int() int64 { panic(values.UnexpectedKind(semantic.Object, semantic.Int)) } -func (t TableObject) UInt() uint64 { +func (t *TableObject) UInt() uint64 { panic(values.UnexpectedKind(semantic.Object, semantic.UInt)) } -func (t TableObject) Float() float64 { +func (t *TableObject) Float() float64 { panic(values.UnexpectedKind(semantic.Object, semantic.Float)) } -func (t TableObject) Bool() bool { +func (t *TableObject) Bool() bool { panic(values.UnexpectedKind(semantic.Object, semantic.Bool)) } -func (t TableObject) Time() values.Time { +func (t *TableObject) Time() values.Time { panic(values.UnexpectedKind(semantic.Object, semantic.Time)) } -func (t TableObject) Duration() values.Duration { +func (t *TableObject) Duration() values.Duration { panic(values.UnexpectedKind(semantic.Object, semantic.Duration)) } -func (t TableObject) Regexp() *regexp.Regexp { +func (t *TableObject) Regexp() *regexp.Regexp { panic(values.UnexpectedKind(semantic.Object, semantic.Regexp)) } -func (t TableObject) Array() values.Array { +func (t *TableObject) Array() values.Array { panic(values.UnexpectedKind(semantic.Object, semantic.Array)) } -func (t TableObject) Object() values.Object { +func (t *TableObject) Object() values.Object { return t } -func (t TableObject) Equal(rhs values.Value) bool { +func (t *TableObject) Equal(rhs values.Value) bool { if t.Type() != rhs.Type() { return false } @@ -240,37 +324,39 @@ func (t TableObject) Equal(rhs values.Value) bool { } return true } -func (t TableObject) Function() values.Function { +func (t *TableObject) Function() values.Function { panic(values.UnexpectedKind(semantic.Object, semantic.Function)) } -func (t TableObject) Get(name string) (values.Value, bool) { +func (t *TableObject) Get(name string) (values.Value, bool) { switch name { - case tableIDKey: - return values.NewStringValue(string(t.ID)), true case tableKindKey: return values.NewStringValue(string(t.Kind)), true case tableParentsKey: return t.Parents, true default: - return nil, false + return t.args.Get(name) } } -func (t TableObject) keys() []string { - return []string{tableIDKey, tableKindKey, tableParentsKey} +func (t *TableObject) keys() []string { + tableKeys := make([]string, 0, len(t.args.GetAll())+2) + return append(tableKeys, tableParentsKey, tableParentsKey) } -func (t TableObject) Set(name string, v values.Value) { - //TableObject is immutable +func (t *TableObject) Set(name string, v values.Value) { + // immutable } -func (t TableObject) Len() int { - return 3 +func (t *TableObject) Len() int { + return len(t.keys()) } -func (t TableObject) Range(f func(name string, v values.Value)) { - f(tableIDKey, values.NewStringValue(string(t.ID))) +func (t *TableObject) Range(f func(name string, v values.Value)) { + for _, arg := range t.args.GetAll() { + val, _ := t.args.Get(arg) + f(arg, val) + } f(tableKindKey, values.NewStringValue(string(t.Kind))) f(tableParentsKey, t.Parents) } @@ -291,23 +377,13 @@ func BuiltIns() (map[string]values.Value, semantic.DeclarationScope) { if !finalized { panic("builtins not finalized") } - qd := new(queryDomain) - return builtIns(qd) + return builtIns() } -func builtIns(qd *queryDomain) (map[string]values.Value, semantic.DeclarationScope) { +func builtIns() (map[string]values.Value, semantic.DeclarationScope) { decls := builtinDeclarations.Copy() scope := make(map[string]values.Value, len(builtinScope)) for k, v := range builtinScope { - if v.Type().Kind() == semantic.Function { - if f, ok := v.Function().(*function); ok { - // Must make separate copy of f. Otherwise function - // tests will modify same f and tests will break. - newfunc := f.copy() - newfunc.qd = qd - v = newfunc - } - } scope[k] = v } interpScope := interpreter.NewScopeWithValues(scope) @@ -329,13 +405,11 @@ func builtIns(qd *queryDomain) (map[string]values.Value, semantic.DeclarationSco } type Administration struct { - id OperationID parents values.Array } -func newAdministration(id OperationID) *Administration { +func newAdministration() *Administration { return &Administration{ - id: id, // TODO(nathanielc): Once we can support recursive types change this to, // interpreter.NewArray(TableObjectType) parents: values.NewArray(semantic.EmptyObject), @@ -348,7 +422,7 @@ func (a *Administration) AddParentFromArgs(args Arguments) error { if err != nil { return err } - p, ok := parent.(TableObject) + p, ok := parent.(*TableObject) if !ok { return fmt.Errorf("argument is not a table object: got %T", parent) } @@ -358,11 +432,11 @@ func (a *Administration) AddParentFromArgs(args Arguments) error { // AddParent instructs the evaluation Context that a new edge should be created from the parent to the current operation. // Duplicate parents will be removed, so the caller need not concern itself with which parents have already been added. -func (a *Administration) AddParent(np TableObject) { +func (a *Administration) AddParent(np *TableObject) { // Check for duplicates found := false - a.parents.Range(func(i int, p values.Value) { - if p.(TableObject).ID == np.ID { + a.parents.Range(func(i int, v values.Value) { + if p, ok := v.(*TableObject); ok && p == np { found = true } }) @@ -371,57 +445,16 @@ func (a *Administration) AddParent(np TableObject) { } } -type Domain interface { - ToSpec() *Spec -} - -func NewDomain() Domain { - return new(queryDomain) -} - -type queryDomain struct { - id int - - operations []TableObject -} - -func (d *queryDomain) NewID(name string) OperationID { - return OperationID(fmt.Sprintf("%s%d", name, d.nextID())) -} - -func (d *queryDomain) nextID() int { - id := d.id - d.id++ - return id -} - -func (d *queryDomain) ToSpec() *Spec { - spec := new(Spec) - visited := make(map[OperationID]bool) - for _, t := range d.operations { - t.buildSpec(spec, visited) - } - return spec -} - type function struct { name string t semantic.Type createOpSpec CreateOperationSpec - qd *queryDomain hasSideEffect bool } -func (f *function) copy() *function { - newfunc := new(function) - *newfunc = *f - return newfunc -} - func (f *function) Type() semantic.Type { return f.t } - func (f *function) Str() string { panic(values.UnexpectedKind(semantic.Function, semantic.String)) } @@ -471,29 +504,19 @@ func (f *function) Call(argsObj values.Object) (values.Value, error) { } func (f *function) call(args interpreter.Arguments) (values.Value, error) { - id := f.qd.NewID(f.name) - - a := newAdministration(id) - - spec, err := f.createOpSpec(Arguments{Arguments: args}, a) + a := newAdministration() + arguments := Arguments{Arguments: args} + spec, err := f.createOpSpec(arguments, a) if err != nil { return nil, err } - if a.parents.Len() > 1 { - // Always add parents in a consistent order - a.parents.Sort(func(i, j values.Value) bool { - return i.(TableObject).ID < j.(TableObject).ID - }) - } - - t := TableObject{ - ID: id, + t := &TableObject{ + args: arguments, Kind: spec.Kind(), Spec: spec, Parents: a.parents, } - f.qd.operations = append(f.qd.operations, t) return t, nil } diff --git a/query/functions/join.go b/query/functions/join.go index 1115807792..f091673988 100644 --- a/query/functions/join.go +++ b/query/functions/join.go @@ -30,6 +30,8 @@ type JoinOpSpec struct { // TODO(nathanielc): Change this to a map of parent operation IDs to names. // Then make it possible for the transformation to map operation IDs to parent IDs. TableNames map[query.OperationID]string `json:"table_names"` + // tableNames maps each TableObject being joined to its unique OperationID + tableNames map[*query.TableObject]string } var joinSignature = semantic.FunctionSignature{ @@ -62,6 +64,7 @@ func createJoinOpSpec(args query.Arguments, a *query.Administration) (query.Oper spec := &JoinOpSpec{ Fn: fn, TableNames: make(map[query.OperationID]string), + tableNames: make(map[*query.TableObject]string), } if array, ok, err := args.GetArray("on", semantic.String); err != nil { @@ -89,9 +92,9 @@ func createJoinOpSpec(args query.Arguments, a *query.Administration) (query.Oper err = fmt.Errorf("value for key %q in tables must be an table object: got %v", k, t.Type()) return } - p := t.(query.TableObject) + p := t.(*query.TableObject) a.AddParent(p) - spec.TableNames[p.ID] = k + spec.tableNames[p] = k }) if err != nil { return nil, err @@ -101,6 +104,12 @@ func createJoinOpSpec(args query.Arguments, a *query.Administration) (query.Oper return spec, nil } +func (t *JoinOpSpec) IDer(ider query.IDer) { + for p, k := range t.tableNames { + t.TableNames[ider.ID(p)] = k + } +} + func newJoinOp() query.OperationSpec { return new(JoinOpSpec) } diff --git a/query/interpreter/interpreter.go b/query/interpreter/interpreter.go index 383779f578..0f2932f26a 100644 --- a/query/interpreter/interpreter.go +++ b/query/interpreter/interpreter.go @@ -824,6 +824,7 @@ func ToStringArray(a values.Array) ([]string, error) { // whether the argument was specified and any errors about the argument type. // semantic.The GetRequired{Type} methods return only two values, the typed value of the arg and any errors, a missing argument is considered an error in this case. type Arguments interface { + GetAll() []string Get(name string) (values.Value, bool) GetRequired(name string) (values.Value, error) @@ -862,6 +863,14 @@ func NewArguments(obj values.Object) Arguments { return newArguments(obj) } +func (a *arguments) GetAll() []string { + args := make([]string, 0, a.obj.Len()) + a.obj.Range(func(name string, v values.Value) { + args = append(args, name) + }) + return args +} + func (a *arguments) Get(name string) (values.Value, bool) { a.used[name] = true v, ok := a.obj.Get(name) diff --git a/query/querytest/compile.go b/query/querytest/compile.go index a106c44dfd..cdea2dddc4 100644 --- a/query/querytest/compile.go +++ b/query/querytest/compile.go @@ -1,6 +1,7 @@ package querytest import ( + "github.com/influxdata/platform/query/functions" "context" "testing" @@ -20,7 +21,9 @@ type NewQueryTestCase struct { var opts = append( semantictest.CmpOptions, cmp.AllowUnexported(query.Spec{}), + cmp.AllowUnexported(functions.JoinOpSpec{}), cmpopts.IgnoreUnexported(query.Spec{}), + cmpopts.IgnoreUnexported(functions.JoinOpSpec{}), ) func NewQueryTestHelper(t *testing.T, tc NewQueryTestCase) { diff --git a/query/querytest/operations.go b/query/querytest/operations.go index ba56de5e2e..66ef85a8c9 100644 --- a/query/querytest/operations.go +++ b/query/querytest/operations.go @@ -5,20 +5,28 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/influxdata/platform/query" + "github.com/influxdata/platform/query/functions" "github.com/influxdata/platform/query/semantic/semantictest" ) func OperationMarshalingTestHelper(t *testing.T, data []byte, expOp *query.Operation) { t.Helper() + opts := append( + semantictest.CmpOptions, + cmp.AllowUnexported(functions.JoinOpSpec{}), + cmpopts.IgnoreUnexported(functions.JoinOpSpec{}), + ) + // Ensure we can properly unmarshal a spec gotOp := new(query.Operation) if err := json.Unmarshal(data, gotOp); err != nil { t.Fatal(err) } - if !cmp.Equal(gotOp, expOp, semantictest.CmpOptions...) { - t.Errorf("unexpected operation -want/+got %s", cmp.Diff(expOp, gotOp, semantictest.CmpOptions...)) + if !cmp.Equal(gotOp, expOp, opts...) { + t.Errorf("unexpected operation -want/+got %s", cmp.Diff(expOp, gotOp, opts...)) } // Marshal the spec and ensure we can unmarshal it again. @@ -31,7 +39,7 @@ func OperationMarshalingTestHelper(t *testing.T, data []byte, expOp *query.Opera t.Fatal(err) } - if !cmp.Equal(gotOp, expOp, semantictest.CmpOptions...) { - t.Errorf("unexpected operation after marshalling -want/+got %s", cmp.Diff(expOp, gotOp, semantictest.CmpOptions...)) + if !cmp.Equal(gotOp, expOp, opts...) { + t.Errorf("unexpected operation after marshalling -want/+got %s", cmp.Diff(expOp, gotOp, opts...)) } } diff --git a/query/repl/repl.go b/query/repl/repl.go index 81f740180a..ea16192d86 100644 --- a/query/repl/repl.go +++ b/query/repl/repl.go @@ -178,7 +178,7 @@ func (r *REPL) executeLine(t string, expectYield bool) (values.Value, error) { // Check for yield and execute query if v.Type() == query.TableObjectType { - t := v.(query.TableObject) + t := v.(*query.TableObject) if !expectYield || (expectYield && t.Kind == functions.YieldKind) { spec := t.ToSpec() // Do query