opa icon indicating copy to clipboard operation
opa copied to clipboard

Clarify behavior of negated expressions

Open tsandall opened this issue 4 years ago • 9 comments

Negated expressions can be produce surprising results if they contain nested terms that require rewriting. For example, the following query will be undefined if input.x is undefined:

not f(input.x)

Most users would expect the query to be defined because they expect not to apply to all of the statements contained on the line.

The reason the result is undefined is that the query is rewritten as temp = input.x; not f(temp). When the first expression evaluates, it fails and the result becomes undefined.

We should take steps to fix this behavior (which would mean a backwards incompatible change) or document the behavior and the rationale for it.

tsandall avatar Nov 06 '19 15:11 tsandall

Is this the cause for equality/inequality comparison with undefined giving unexpected results?


Comparing two unique values for inequality when both are defined, as expected result: true:

input.a != input.b

If either in above are undefined, as expected the result: undefined.


Comparing two unique values for inequality if only one is undefined, as expected result: true:

not input.a == input.b

If both values are undefined the above will unexpectedly result: true.


Given the above, is this the only way to check for inequality?:

inequality {
    input.a != input.b
}

inequality {
    not input.a
    input.b
}

inequality {
    input.a
    not input.b
}

ryanbeau avatar Dec 17 '19 17:12 ryanbeau

Given the above, is this the only way to check for inequality?

If you want inequality defined the way I think you're expecting it to be, you can write not input.a == input.b inside of rules. For example: https://play.openpolicyagent.org/p/j7t0gqTECa. That works because == is lowered to = (unification) which does not require rewriting like the built-in functions (!=, >, +, count(...), etc.)

tsandall avatar Dec 20 '19 18:12 tsandall

My comment was meant to confirm if it was intentional for not undefined == undefined to result in true.

All the other given scenarios give the logical output of true except for when they're both undefined. Otherwise relying solely on not input.a == input.b is not reliable in the situation when neither are defined.

ryanbeau avatar Dec 20 '19 18:12 ryanbeau

not input.a == input.b is true/defined when either (or both) input.a and input.b are missing/undefined, i.e., not undefined == undefined is true. not is how you can catch undefined inside of Rego.

It's possible you ran into some confusing behaviour in the REPL due to == vs =:

> not input.a = input.b  # assume input.a and input.b are missing
true
> not input.a == input.b
undefined  # expectation: true

This behaviour is a bit confusing because if you put not input.a == input.b inside a rule body you get the expected behaviour (note: == not =):

> show
package repl

p {
	not input.a == input.b
}
> p
true

The strange REPL behaviour (i.e., not input.a == input.b => undefined) is due to a decision to produce true/false outputs as much as possible. This was done to make people coming from an imperative background more comfortable. For example, we found people got quite confused when a statement like 1 == 2 printed undefined instead of false. To implement this, OPA does not lower == to = in the REPL. You can see this if you enable tracing:

> trace
> input.a == input.b
Enter __localq0__ = input.a; __localq1__ = input.b; equal(__localq0__, __localq1__, _)

The last expression (equal(__localq0__, __localq1__, _) checks if two vars are equal and saves the result in another variable _. In this case, == (or equal(a,b) is not lowered to =.

Writing this up, I've realized we could resolve the confusing REPL (not input.a == input.b => undefined) by allowing the expression to get lowered to =. Since the expression is negated there can be no output variables (so this is safe). This way we'd get the following:

> input.a = input.b
undefined
> not input.a = input.b
true
> input.a == input.b
undefined
> not input.a == input.b  # today this prints 'undefined'
true 

tsandall avatar Jan 09 '20 14:01 tsandall

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

stale[bot] avatar Nov 22 '21 22:11 stale[bot]

I just ran into this issue and was about to file a new bug until I found this issue. My two cents is that not f(input.x) should evaluate to true.

Otherwise, I don't see how we are going to rationally define/explain the behavior to say that rules a and b below have different outcome:

a {
  not (input.x + 1)
}

var_1 := (input.x + 1)
b {
  not var_1
}

c {
  var_1 := (input.x + 1)
  not var_1
}

It is also somewhat less than ideal that a and c disagree, but I think that's easier to explain sensibly.

I also see how it's not a simple decision to say exactly how to proceed when the behavior has already been around for quite a while.

ericjkao avatar Jan 27 '22 03:01 ericjkao

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

stale[bot] avatar Feb 26 '22 03:02 stale[bot]

Noting another "crazy" consequence of this behavior:

Assume input.x is undefined. Then a succeeds but b fails in the rules below because the second one gets re-written with a separate variable assignment the value of input.x.

a {  # succeeds
  not input.x == 1
}

b {  # fails
  not input.x >= 1
}

ericjkao avatar Jul 25 '22 22:07 ericjkao

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

stale[bot] avatar Aug 30 '22 22:08 stale[bot]

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

stale[bot] avatar Oct 27 '22 21:10 stale[bot]