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 null handling in FunctionInvocation #539

Closed
Tracked by #67
nikku opened this issue Jan 16, 2023 · 6 comments · Fixed by #612
Closed
Tracked by #67

Clarify null handling in FunctionInvocation #539

nikku opened this issue Jan 16, 2023 · 6 comments · Fixed by #612

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 / handling of non existing functions FunctionExpression is not clearly specified and may lead to engines behaving slightly different:

// given
value = non_existing_function(10)

Expected Behavior

// then (?)
value = null

Alternative Interpretations

To be clear there exists different possible interpretations:

strict null handling, blowing up

`non_existing_function` is not a function`

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 #536

@StrayAlien
Copy link
Contributor

Hi @nikku - for sure the 'error handling' aspects of the spec leave it all pretty open and up to the impl. What is certain is that invoking a non-existent function is a null. Now, how an impl chooses to report that either at compile time (if indeed, it has a compiler) or runtime is not defined.

Personally, I think that given the broad range of possibilities for implementors to operate as a pure 'dynamic runtime' engine with no compiler to a more advanced 'strict compiler' model would make it very hard for the spec to encompass error codes and messages and so on.

I think it is more a point of differentiation between engines how they handle errors - if (in a perfect world) we all executed in the same way, then you get choice as to how helpful a vendor's toolset is in model authoring or execution.

As a dev, I personally favour the strict compiler approach :-) ... but, others may not.

In TCK terms, a test execution is considered to have an 'error' if any error (such as a missing function), or a type null-coercion takes place. So, loosely, a test can detect an 'error' but it is not very specific.

@nikku
Copy link
Contributor Author

nikku commented Jan 19, 2023

What is certain is that invoking a non-existent function is a null.

@StrayAlien If this is certain then you answered the main question.

I'd be happy to create a TCK test covering that behavior.

nonExistingFunction(1) = null

I agree that the spec leaves it open how error handling is done (result is null but there can be an error still). The purpose would be to cover execution from the pure language semantics (not error handling) perspective.

@baldimir
Copy link
Collaborator

Hi @nikku there were those two PRs from you. Are those solving also this issue please? If yes, could we close it please?

@nikku
Copy link
Contributor Author

nikku commented Jun 15, 2023

To my knowledge this one is still not verified by the TCK. I hope to find time to file a PR with respective test cases.

@opatrascoiu
Copy link
Contributor

Hi @nikku

I think the DMN spec covers your use case. According to DMN spec (see 10.3.2.13.2)

If f is not a function, or if the arguments do not conform to the argument list, the result of the invocation is null.

the result is null.

If you are planning to write tests, please create them in a new test file to cover section 10.3.2.13.1 and 10.3.2.13.2 (e.g. XXXX-feel-function-invocation).

Thank you.

nikku added a commit to camunda/dmn-tck that referenced this issue Jul 3, 2023
@nikku
Copy link
Contributor Author

nikku commented Jul 3, 2023

This is being validated via #612.

nikku added a commit to camunda/dmn-tck that referenced this issue Jul 3, 2023
nikku added a commit to camunda/dmn-tck that referenced this issue Jul 3, 2023
nikku added a commit to camunda/dmn-tck that referenced this issue Jul 3, 2023
nikku added a commit to nikku/feelin that referenced this issue Jul 3, 2023
Skaiir added a commit to bpmn-io/feelers that referenced this issue Jul 10, 2023
Skaiir added a commit to bpmn-io/feelers that referenced this issue Jul 11, 2023
baldimir pushed a commit that referenced this issue Jul 20, 2023
* test: verify non-existing function invocation

Yields `null`.

Closes #539

* test: verify additional function invocation cases
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

Successfully merging a pull request may close this issue.

4 participants