From b2f5e70fcdc7bc07cbe5218fecbcec87835ba7cc Mon Sep 17 00:00:00 2001 From: Torin Sandall Date: Tue, 7 Feb 2017 15:13:00 -0800 Subject: [PATCH] Allow sets to be treated like objects/arrays In the past, topdown would bind set[x] to true. This resulted in confusion when people attempted to write joins with set elements. With this change, topdown treats sets like objects/arrays, except that set[x] is bound to x. This allows users to write queries that dereference sets just like objects and arrays. Also, fix handling of self-joins where previously a recursive binding could be added. The "self-join" test case was added to cover this. Fixes #243 --- repl/repl_test.go | 2 +- .../working-with-the-opa-repl/index.md | 4 +- topdown/errors.go | 8 --- topdown/topdown.go | 61 +++++++++---------- topdown/topdown_test.go | 18 ++++-- 5 files changed, 44 insertions(+), 49 deletions(-) diff --git a/repl/repl_test.go b/repl/repl_test.go index 1259d62be8..4e9aa3da2a 100644 --- a/repl/repl_test.go +++ b/repl/repl_test.go @@ -619,7 +619,7 @@ func TestEvalSingleTermMultiValueSetRef(t *testing.T) { // acceptable, as it should be an edge case. buffer.Reset() repl.OneShot(ctx, "r[_][x]") - expected = parseJSON(`[{"x": 5, "r[_][x]": true}, {"x": 6, "r[_][x]": true}, {"x": 0, "r[_][x]": 7}, {"x": 1, "r[_][x]": 8}]`) + expected = parseJSON(`[{"x": 5, "r[_][x]": 5}, {"x": 6, "r[_][x]": 6}, {"x": 0, "r[_][x]": 7}, {"x": 1, "r[_][x]": 8}]`) result = parseJSON(buffer.String()) if !reflect.DeepEqual(result, expected) { t.Fatalf("Expected %v but got: %v", expected, result) diff --git a/site/examples/working-with-the-opa-repl/index.md b/site/examples/working-with-the-opa-repl/index.md index 1261cec96a..8e6535c887 100644 --- a/site/examples/working-with-the-opa-repl/index.md +++ b/site/examples/working-with-the-opa-repl/index.md @@ -100,7 +100,7 @@ In addition to running queries, the REPL also lets you define rules: ```ruby > p[x] { a = [1,2,3,4], a[x] } -> p[x], x > 1 +> p[x] > 1 +---+ | x | +---+ @@ -247,7 +247,7 @@ The REPL also understands the [Import and Package](/documentation/how-do-i-write ```ruby > package opa.example -> public_servers[x], x.protocols[_] = "http" +> public_servers[x].protocols[_] = "http" +-------------------------------------------------------------------+ | x | +-------------------------------------------------------------------+ diff --git a/topdown/errors.go b/topdown/errors.go index bf9b4a2f12..72254b779a 100644 --- a/topdown/errors.go +++ b/topdown/errors.go @@ -76,11 +76,3 @@ func objectDocKeyTypeErr(loc *ast.Location) error { Message: "partial rule definitions must produce string values for object keys", } } - -func setDereferenceTypeErr(loc *ast.Location) error { - return &Error{ - Code: TypeErr, - Location: loc, - Message: "set documents cannot be dereferenced", - } -} diff --git a/topdown/topdown.go b/topdown/topdown.go index 06e63aeb09..50f271dfe6 100644 --- a/topdown/topdown.go +++ b/topdown/topdown.go @@ -385,19 +385,6 @@ func Continue(t *Topdown, key, value ast.Value, iter Iterator) error { return err } -// ContinueN binds N keys to N values. The key/value pairs are passed in as -// alternating pairs, e.g., key-1, value-1, key-2, value-2, ..., key-N, value-N. -func ContinueN(t *Topdown, iter Iterator, x ...ast.Value) error { - var prev *Undo - for i := 0; i < len(x)/2; i++ { - offset := i * 2 - prev = t.Bind(x[offset], x[offset+1], prev) - } - err := iter(t) - t.Unbind(prev) - return err -} - // Eval evaluates the query in t and calls iter once for each set of bindings // that satisfy all of the expressions in the query. func Eval(t *Topdown, iter Iterator) error { @@ -1251,9 +1238,6 @@ func evalRefRule(t *Topdown, ref ast.Ref, path ast.Ref, rules []*ast.Rule, iter if len(suffix) == 0 { return evalRefRulePartialSetDocFull(t, ref, rules, iter) } - if len(suffix) != 1 { - return setDereferenceTypeErr(t.Current().Location) - } for i, rule := range rules { err := evalRefRulePartialSetDoc(t, ref, path, rule, i > 0, iter) if err != nil { @@ -1570,8 +1554,8 @@ func evalRefRulePartialSetDoc(t *Topdown, ref ast.Ref, path ast.Ref, rule *ast.R return fmt.Errorf("unbound variable: %v", rule.Head.Value) } child.traceExit(rule) - undo, err := evalEqUnify(t, key, value, nil, func(child *Topdown) error { - return Continue(t, ref[:len(path)+1], ast.Boolean(true), iter) + undo, err := evalEqUnify(t, key, value, nil, func(t *Topdown) error { + return evalRefRuleResult(t, ref, ref[len(path)+1:], PlugValue(key, t.Binding), iter) }) if err != nil { @@ -1593,7 +1577,7 @@ func evalRefRulePartialSetDoc(t *Topdown, ref ast.Ref, path ast.Ref, rule *ast.R } return eval(child, func(child *Topdown) error { child.traceExit(rule) - err := Continue(t, ref[:len(path)+1], ast.Boolean(true), iter) + err := evalRefRuleResult(t, ref, ref[len(path)+1:], key, iter) if err != nil { return err } @@ -1644,9 +1628,22 @@ func evalRefRuleResult(t *Topdown, ref ast.Ref, suffix ast.Ref, result ast.Value } return evalRefRuleResultRec(t, result, s, ast.Ref{}, func(t *Topdown, v ast.Value) error { + + // Check if we already have binding for ref. This can happen when + // evaluating self-joins. + plugged := make(ast.Ref, len(ref)) + + for i := range plugged { + plugged[i] = PlugTerm(ref[i], t.Binding) + } + + if t.Binding(plugged) != nil { + return iter(t) + } + // Must add binding with plugged value of ref in case ref contains - // suffix with one or more vars. - // Test case: "input: object dereference ground 2" exercises this. + // suffix with one or more vars. Test "input: object dereference + // ground 2" exercises this. return Continue(t, PlugValue(ref, t.Binding), v, iter) }) } @@ -1660,10 +1657,10 @@ func evalRefRuleResultRec(t *Topdown, v ast.Value, ref, path ast.Ref, iter func( switch v := v.(type) { case ast.Array: return evalRefRuleResultRecArray(t, v, ref, path, iter) - case *ast.Set: - return evalRefRuleResultRecSet(t, v, ref, path, iter) case ast.Object: return evalRefRuleResultRecObject(t, v, ref, path, iter) + case *ast.Set: + return evalRefRuleResultRecSet(t, v, ref, path, iter) case ast.Ref: return evalRefRuleResultRecRef(t, v, ref, path, iter) } @@ -1749,19 +1746,17 @@ func evalRefRuleResultRecRef(t *Topdown, v, ref, path ast.Ref, iter func(*Topdow }) } -func evalRefRuleResultRecSet(t *Topdown, set *ast.Set, ref, suffix ast.Ref, iter func(*Topdown, ast.Value) error) error { +func evalRefRuleResultRecSet(t *Topdown, set *ast.Set, ref, path ast.Ref, iter func(*Topdown, ast.Value) error) error { head, tail := ref[0], ref[1:] - if len(tail) > 0 { - return nil - } switch k := head.Value.(type) { case ast.Var: for _, e := range *set { undo := t.Bind(k, e.Value, nil) - err := iter(t, ast.Boolean(true)) - if err != nil { + path = append(path, e) + if err := evalRefRuleResultRec(t, e.Value, tail, path, iter); err != nil { return err } + path = path[:len(path)-1] t.Unbind(undo) } return nil @@ -1780,10 +1775,12 @@ func evalRefRuleResultRecSet(t *Topdown, set *ast.Set, ref, suffix ast.Ref, iter return err } - if rset.Contains(ast.NewTerm(rval)) { - return iter(t, ast.Boolean(true)) + if !rset.Contains(ast.NewTerm(rval)) { + return nil } - return nil + + path = append(path, head) + return evalRefRuleResultRec(t, rval, tail, path, iter) } } diff --git a/topdown/topdown_test.go b/topdown/topdown_test.go index bae17546ad..65dba3b80b 100644 --- a/topdown/topdown_test.go +++ b/topdown/topdown_test.go @@ -477,10 +477,10 @@ func TestTopDownVirtualDocs(t *testing.T) { }{ // input to partial set and object docs {"input: set 1", []string{"p = true :- q[1]", "q[x] :- a[i] = x"}, "true"}, - {"input: set 2", []string{"p[x] :- q[1] = x", "q[x] :- a[i] = x"}, "[true]"}, - {"input: set embedded", []string{`p[x] :- x = {"b": [q[2]]}`, `q[x] :- a[i] = x`}, `[{"b": [true]}]`}, + {"input: set 2", []string{"p[x] :- q[1] = x", "q[x] :- a[i] = x"}, "[1]"}, + {"input: set embedded", []string{`p[x] :- x = {"b": [q[2]]}`, `q[x] :- a[i] = x`}, `[{"b": [2]}]`}, {"input: set undefined", []string{"p = true :- q[1000]", "q[x] :- a[x] = y"}, ""}, - {"input: set dereference error", []string{"p :- x = [1], q[x][0]", "q[[x]] :- a[_] = x"}, setDereferenceTypeErr(nil)}, + {"input: set dereference", []string{"p = y :- x = [1], q[x][0] = y", "q[[x]] :- a[_] = x"}, "1"}, {"input: set ground var", []string{"p[x] :- x = 1, q[x]", "q[y] :- a[y] = i"}, "[1]"}, {"input: set ground composite (1)", []string{ "p :- z = [[1,2], 2], q[z]", @@ -517,9 +517,11 @@ func TestTopDownVirtualDocs(t *testing.T) { // output from partial set and object docs {"output: set", []string{"p[x] :- q[x]", "q[y] :- a[i] = y"}, "[1,2,3,4]"}, - {"output: set embedded", []string{`p[i] :- {i: [true]} = {i: [q[i]]}`, `q[x] :- d.e[i] = x`}, `["bar", "baz"]`}, + {"output: set embedded", []string{`p[i] :- {i: [i]} = {i: [q[i]]}`, `q[x] :- d.e[i] = x`}, `["bar", "baz"]`}, {"output: set var binding", []string{"p[x] :- q[x]", "q[y] :- y = [i, j], i = 1, j = 2"}, `[[1,2]]`}, - {"output: set dereference error", []string{"p :- q[x][0]", "q[[x]] :- a[_] = x"}, setDereferenceTypeErr(nil)}, + {"output: set dereference", []string{"p[y] :- q[x][0] = y", "q[[x]] :- a[_] = x"}, `[1,2,3,4]`}, + {"output: set dereference deep", []string{"p[y] :- q[i][j][k][x] = y", "q[{{[1],[2]},{[3],[4]}}] :- true"}, "[1,2,3,4]"}, + {"output: set falsy values", []string{"p[x] :- q[x]", `q = {0, "", false, null, [], {}, set()} :- true`}, `[0, "", null, [], {}, []]`}, {"output: object key", []string{"p[x] :- q[x] = 4", "q[i] = x :- a[i] = x"}, "[3]"}, {"output: object value", []string{"p[x] = y :- q[x] = y", "q[k] = v :- b[k] = v"}, `{"v1": "hello", "v2": "goodbye"}`}, {"output: object embedded", []string{"p[k] = v :- {k: [q[k]]} = {k: [v]}", `q[x] = y :- b[x] = y`}, `{"v1": "hello", "v2": "goodbye"}`}, @@ -548,6 +550,10 @@ func TestTopDownVirtualDocs(t *testing.T) { "p[z] :- q[x] = y, z = [x, y]", `q[k] = v :- k = y, y = x, x = "a", v = "foo"`}, `[["a", "foo"]]`}, + {"object: self-join", []string{ + "p[[x, y]] :- q[x] = 1, q[y] = x", + "q[x] = i :- a[i] = x"}, + "[[2,3]]"}, // input+output from partial set/object docs {"i/o: objects", []string{ @@ -837,7 +843,7 @@ func TestTopDownVarReferences(t *testing.T) { {"set: ground var", []string{"p[x] :- i = {1,2,3,4}, j = {1,2,99}, j[x], i[x]"}, "[1,2]"}, {"set: lookup: base docs", []string{`p :- v = {[1,999],[3,4]}, pair = [a[2], 4], v[pair]`}, "true"}, {"set: lookup: embedded", []string{"p :- x = [{}, {[1,2], [3,4]}], y = [3,4], x[i][y]"}, "true"}, - {"set: lookup: dereference: undefined", []string{"p :- x = [{}, {[1,2], [3,4]}], y = [3,4], x[i][y][z]"}, ""}, + {"set: lookup: dereference", []string{"p[[i,z,r]] :- x = [{}, {[1,2], [3,4]}], y = [3,4], x[i][y][z] = r"}, "[[1,0,3], [1,1,4]]"}, {"avoids indexer", []string{"p = true :- somevar = [1,2,3], somevar[i] = 2"}, "true"}, }