Handle non-existing variables and context entries gracefully in comparison and filter expressions

[saig0]

Describe the bug
If I compare a non-existing variable with a value then the expression fails. To avoid failure, I must do a null check first.

x = 3
// failed to evaluate expression ' x = 3 ': no variable found for name 'x'

x > 3
// failed to evaluate expression ' x > 3 ': ValError(no variable found for name 'x') is not comparable

x != null and x = 3
// returns false

The behavior is the same for filter expressions.

[{x:5},{y:2}][x > 3]
// failed to evaluate expression ' [{x:5},{y:2}][x > 3]  ': ValError(no variable found for name 'x') is not comparable)

[{x:5},{y:2}][x != null and x > 3]
// returns [{x:5}]

Related discussion.
Related to #572.
Related to #260.

To Reproduce
Steps to reproduce the behavior:

1. Evaluate the expression x = 5
2. Verify that the expression is not successful

Expected behavior
A non-existing variable or context entry is handled gracefully in a comparison and a filter.

Instead of failing the evaluation, the comparison or filter handles the non-existing value as null. The comparison may return false or null. The filter doesn't include the list item in the result.

Since the engine handles the case gracefully, no warning should be printed.

(null = 3) = false

(null < 3) = null

// 10.3.2.5 - example 3
[ { x: 1, y: 2 }, { x: null, y: 3 } ][ x < 2 ] = [ { x: 1, y: 2 } ]

// non-member matching
[ { x: 1 } ][ y > 1 ] = [ ]

[ { x: 1 }, { y: 2 } ][ y > 1 ] = [ { y: 2 } ]

Environment

• FEEL engine version: 1.15.2
• Affects:
  • Camunda Automation Platform 7: [7.x]
  • Zeebe broker: [0.x]

[saig0]

Related to the issue dmn-tck/tck#536 that points to the following section of the DMN spec:

[cjcalmeida]

I have a question about this topic:

We have an implementation that validates the expression (if the expression is valid and if the variables exist in the context) in standalone way and we use FeelEngine (evalUnaryTests or evalExpression) to do it, but today FeelEngine doesn't fail, it just emits a log warn level.

When we run the complete DMN (when we put expression in DMN and run Camunda), the evaluate of the expression fails, because the variable does not exist.

Wouldn't it be better to have the same result in both cases?

I use Camunda Platform 7.17 (DMN 7.17 too)

[saig0]

Wouldn't it be better to have the same result in both cases?

Yes. The FEEL engine should behave the same. However, the DMN (or BPMN) engine may raise an error (or incident) if the expression doesn't return a required value.

It would be great if you could share your example. Happy to discuss it in detail.

[korthout]

@saig0, the ZPA team would like to work on this together with you. Do you think you would have time for that, or is there any way we can clarify what needs to happen here?

[saig0]

@korthout awesome. Let's schedule a pair/mob programming session (but not before 2023-04-24). We should plan for at least 2 hours of coding.

[korthout]

@saig0 I'll be unavailable for about two weeks starting April 26th. You may need to organize this with the team, or someone else from @camunda/zeebe-process-automation needs to arrange this.

[saig0]

The expected behavior is now clarified. There is a new TCK test case for it.

[korthout]

That's great! Perhaps we can tackle this in the first mob programming session after the retreat.

//cc @remcowesterhoud

[saig0]

⚠️ Note that we change the behavior of less/greater than comparison with null. Previously, the comparison returned false (see #50). Now, the comparison will return null instead which is aligned with the DMN spec. Only two values of the same type are comparable.

For example:

null < 5
// 1.16: false
// 1.17: null

non_existing < 5
// 1.16: failure
// 1.17: null

null between 1 and 10
// 1.16: false
// 1.17: null   

non_existing between 1 and 10
// 1.16: null
// 1.17: null

---

Side note: The DMN spec says that null should be used for NaN, positive and negative infinity. But null can't be used to compare with a number. So, it is not possible to check against infinity. In practice, this should not be relevant.

[remcowesterhoud]

Now that #676 is merged we have "completed" the non-existing variables in comparisons part. The filter expressions still need to be done!

Whilst working on #676 I found 2 bugs:

• Null in unary-test expressions should resolve to ValNull #679
• Reserved key words can be used as variables #681

[korthout]

The team spend time on this during our weekly mob programming session, resulting in:

• feat: Improve null handling in filter expressions #685

We'll continue it again next week

[korthout]

Moving this back to Ready, as there is one step left:

• null handling in filter expressions with contexts (see examples in issue description)