Bug in partial evaluation: self-comparison missing

[johanneskra]

Short description

There seems to be a bug in the partial evaluation when doing self-comparisons. Self-comparisons are missing in the result.

Steps To Reproduce

1. Create a file named data.json with the following content:

{
  "a": "hello"
}

2. The normal evaluation works as expected:

opa eval --input data.json --format=pretty 'input.a == input.a'
# > output: true

opa eval --format=pretty 'input.a == input.a'
# > output: undefined

As you can see, even though input.a is compared with itself, it is not guaranteed to always return true.
The evaluation returns undefined if a is not supplied.

3. However, in the partial evaluation, this is not taken into consideration. The condition input.a == input.a is missing from the result:

opa eval --format=pretty --partial 'input.a == input.a' --unknowns input.a
# > output: "| Query 1 |  |"

Expected behavior

I would expect the condition input.a == input.a to be in the query result since it might be false/undefined.

So the expected output would be:

| Query 1 | input.a = input.a |

Additional context

I tested this with OPA v0.41.0.

[srenatus]

Thanks for the report!

[srenatus]

One could argue that the use of input.a == input.a in a policy is rather uncommon. Semantically, it's the same as saying _ = input.a, which would (I hope) not get eaten during PE. I'd using that instead an option for you, as a workaround?

That said, PE should not drop meaningful queries, that's for sure, so the bug is valid. It just strikes me as avoidable. 🤔

[srenatus]

FWIW I suspect it's copy propagation, but there's no way to quickly try this just using opa eval

[srenatus]

📜 Just some notes, thinking aloud

Looks like the bug is two-fold: copyprop fails to recognize the necessity to keep something for input.a == input.a, but also partially evaluating a policy,

package p
a { input.a == input.a }

eliminates it -- because when that calls copyprop, it doesn't pass input as livevar. 🔍

💡 When input.a == input.a is fed as query into PartialRun, it'll be rewritten to

__localq0__ = input.a
__localq1__ = input.a
equal(__localq0__, __localq1__)

and for that, the previous copyprop logic was sufficient.