Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rego vs wasm: x == arr[x] #5271

Closed
srenatus opened this issue Oct 19, 2022 · 1 comment · Fixed by #5276
Closed

rego vs wasm: x == arr[x] #5271

srenatus opened this issue Oct 19, 2022 · 1 comment · Fixed by #5276
Assignees
Labels

Comments

@srenatus
Copy link
Contributor

This one is hard to summarise, but the example is clear:

package play

p {
	some foo
	foo == input.foos[foo]
}
echo '{"foos": ["foo1", "foo2"] }' | opa eval -fpretty -d pol.rego -I data.play.p
topdown wasm
undefined true

Note that it's important that the RHS is input.foos, since if the array was known, we'd get a compile time error:

package play

p {
	foos := ["foo1", "foo2"]
	some foo
	foo == foos[foo]
}
$ opa eval -fpretty -d 2e.rego -I data.play.p
1 error occurred: 2e.rego:6: rego_type_error: match error
	left  : number
	right : string
@srenatus srenatus added the bug label Oct 19, 2022
@srenatus srenatus self-assigned this Oct 20, 2022
@srenatus
Copy link
Contributor Author

FWIW it's compiled into that rego

p = true { __local0__ = input.foos[__local0__] }

The planner will plan this:

| *ir.Funcs Funcs (1 funcs)
| | *ir.Func g0.data.test.p (2 params: [Local<0> Local<1>], 3 blocks, path: [g0 test p])
| | | *ir.Block Block (3 statements)
| | | | *ir.ResetLocalStmt &{Target:Local<3> Location:{File:0 Col:1 Row:2 file:module-0.rego text:p}}
| | | | *ir.DotStmt &{Source:{Value:Local<0>} Key:{Value:String<1>} Target:Local<4> Location:{File:0 Col:3 Row:4 file:module-0.rego text:foo == input.foos[foo]}}
| | | | *ir.ScanStmt &{Source:Local<4> Key:Local<5> Value:Local<6> Block:Block (3 statements) Location:{File:0 Col:3 Row:4 file:module-0.rego text:foo == input.foos[foo]}}
| | | | | *ir.Block Block (3 statements)
| | | | | | *ir.AssignVarStmt &{Source:{Value:Local<5>} Target:Local<7> Location:{File:0 Col:3 Row:4 file:module-0.rego text:foo == input.foos[foo]}}
| | | | | | *ir.AssignVarStmt &{Source:{Value:Local<6>} Target:Local<8> Location:{File:0 Col:3 Row:4 file:module-0.rego text:foo == input.foos[foo]}}
| | | | | | *ir.AssignVarOnceStmt &{Source:{Value:Bool<true>} Target:Local<3> Location:{File:0 Col:1 Row:2 file:module-0.rego text:p}}
| | | *ir.Block Block (2 statements)
| | | | *ir.IsDefinedStmt &{Source:Local<3> Location:{File:0 Col:1 Row:2 file:module-0.rego text:p}}
| | | | *ir.AssignVarOnceStmt &{Source:{Value:Local<3>} Target:Local<2> Location:{File:0 Col:1 Row:2 file:module-0.rego text:p}}
| | | *ir.Block Block (1 statements)
| | | | *ir.ReturnLocalStmt &{Source:Local<2> Location:{File:0 Col:1 Row:2 file:module-0.rego text:p}}

The relevant section is

| | | | *ir.ScanStmt &{Source:Local<4> Key:Local<5> Value:Local<6> Block:Block (3 statements) Location:{File:0 Col:3 Row:4 file:module-0.rego text:foo == input.foos[foo]}}
| | | | | *ir.Block Block (3 statements)
| | | | | | *ir.AssignVarStmt &{Source:{Value:Local<5>} Target:Local<7> Location:{File:0 Col:3 Row:4 file:module-0.rego text:foo == input.foos[foo]}}
| | | | | | *ir.AssignVarStmt &{Source:{Value:Local<6>} Target:Local<8> Location:{File:0 Col:3 Row:4 file:module-0.rego text:foo == input.foos[foo]}}
| | | | | | *ir.AssignVarOnceStmt &{Source:{Value:Bool<true>} Target:Local<3> Location:{File:0 Col:1 Row:2 file:module-0.rego text:p}}

which is simply kind of wrong. They key Local<5> is unified with a new var, Local<7> (the first AssignVarStmt). The second one is the result of unifying the value Local<6> with another new var, Local<8>. When the second AssignVarStmt is planned, we should really notice that we don't need a new var but should unify with Local<7>... 🤔

srenatus added a commit to srenatus/opa that referenced this issue Oct 21, 2022
Previously, the planner didn't account for the variable to become known in the
process of planning term b.

In the case here, foo became known when planning the ref `input.foos[foo]`, the
rhs of the `foo = input.foos[foo]` unification.

Fixes open-policy-agent#5271.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
srenatus added a commit that referenced this issue Oct 21, 2022
Previously, the planner didn't account for the variable to become known in the
process of planning term b.

In the case here, foo became known when planning the ref `input.foos[foo]`, the
rhs of the `foo = input.foos[foo]` unification.

Fixes #5271.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant