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

Clarify FEEL null handling in FilterExpression #536

Closed
nikku opened this issue Jan 16, 2023 · 4 comments
Closed

Clarify FEEL null handling in FilterExpression #536

nikku opened this issue Jan 16, 2023 · 4 comments

Comments

@nikku
Copy link
Contributor

nikku commented Jan 16, 2023

Describe the context

The DMN 1.4 specification gives users some pointers on how FEEL should handle invalid inputs and data formats (cf. additional context). From what it reads to me the way is null cohersion. Invalid (incompatible operations, access, or invocations) yield null. That intention is not explicitly expressed though (you need to read between the lines, interpret what is actually specified.

null cohersion around FilterExpression is not clearly specified and may lead to engines behaving slightly different:

// given
list = [
  { a: 1 },
  { a: 2 },
  { b: 1 }
]

// when
result = list[ a < 3 ];

Expected Behavior

// then (?)
result = [ 
  { a: 1 },
  { a: 2 }
]

Alternative Interpretations

To be clear there exists a number of different possible interpretations:

strict null handling, blowing up

a is not present on list entry `{ b: 1 }`

strict type format handling

Coherse to empty list as not all of the list members match { b }.

result = []

Additional Context

I'm aware of the following cases of null cohersion, explicitly defined by the spec:

Access function with wrong arguments

date(1, 2, 3, 4, 5, 6) = null

Unboxing of single value lists

["a"] + "a" = "aa"

Related to #537

@nikku nikku changed the title Clarify FEEL null cohersion in FilterExpression Clarify FEEL null handling in FilterExpression Jan 16, 2023
@barmac
Copy link

barmac commented Jan 17, 2023

I brought this issue up in the DMN 1.5 RTF meeting today. We looked together into the specification and agreed that it's sufficiently specified. On page 109, one can read:

The filter expression is evaluated for each item in list, and a list containing only items where the filter expression is true is returned. E.g:
[ {x:1, y:2}, {x:null, y:3} ][x < 2] = [{x:1, y:2}]

So for the third element, the evaluated filter is:

{ b: 1 }.a < 3

// which is equal to
null < 3

// which results in
null

As null != true, the filter expression should drop the item.

As a follow-up, we should create a PR with a test case which ensures the behavior above between the implementations.

@nikku
Copy link
Contributor Author

nikku commented Jan 18, 2023

Thanks for clarifying @barmac.

I can see if I can add a TCK test case to cover two topics that you mention:

// access of undefined members in a Context
{ x: 1 }.y = null
// filtering of lists on undefined members
[ { x: 1 } ][y > 1] = []

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

@opatrascoiu
Copy link
Contributor

Before adding new tests please check if the existing ones do not cover your use case.

@nikku
Copy link
Contributor Author

nikku commented Jan 19, 2023

Writing test cases for this you could argue that the spec is "underspecified", because each of the provided list member contexts have actual members with the given name.

#540 attempts to create test cases for all cases covered by the spec (10.3.2.5 Lists and filters), and also adds cases discussed in this issue (non-member access).

At the same time #541 asserts the behavior of non-member context access.

nikku added a commit to camunda/dmn-tck that referenced this issue Jan 19, 2023
nikku added a commit to camunda/dmn-tck that referenced this issue Jan 19, 2023
nikku added a commit to camunda/dmn-tck that referenced this issue Jan 19, 2023
nikku added a commit to camunda/dmn-tck that referenced this issue May 24, 2023
nikku added a commit to camunda/dmn-tck that referenced this issue May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants