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

Support undefined keyword #622

Closed
korthout opened this issue Apr 21, 2023 · 2 comments
Closed

Support undefined keyword #622

korthout opened this issue Apr 21, 2023 · 2 comments

Comments

@korthout
Copy link
Member

Is your feature request related to a problem? Please describe.
It may be useful to express the difference between a missing value null and an undefined value undefined. For example, when a FEEL context is used to express a JSON object as the body of a PUT request.

In REST APIs, behaving differently when a value is missing or undefined is common practice. For example, in a PUT request:

  • If the property is null, the PUT request may be interpreted to UPDATE the property's value to null
  • If the property is undefined, the PUT request may be interpreted to ignore the property

Currently with FEEL
One way that I could find to leave out properties of a FEEL context is to build the context dynamically using context put(context, key, value), when the value is defined(value). However, this may be cumbersome to use:

if is defined(y) then context put({}, x, y) else {}

Composing multiple calls becomes tedious, but it can be simplified slightly using a function property.

{
  dynamicPut: function(c, k, v) if is defined(v) then context put(c, k, v) else c
  context: dynamicPut(dynamicPut({}, y, anotherUndefinedVar), x, optionallyUndefinedVar)
}.context

However, this is still more cumbersome than using an undefined keyword:

{
  x: if is defined(optionallyUndefinedVar) then optionallyUndefinedVar else undefined
  y: if is defined(anotherUndefinedVar) then anotherUndefinedVar else undefined
}

Describe the solution you'd like
Add support for an undefined keyword that can be used to indicate that a value is undefined.

is defined(null)
// true

is defined(undefined)
// false

Related issues

@saig0
Copy link
Member

saig0 commented May 16, 2023

@korthout thank you for raising this up. This is an interesting use case. 👍

According to the DMN spec, FEEL doesn't differ between a variable/property that is null or that doesn't exist. In both cases, the FEEL expression returns null. So, the support of a new undefined type doesn't fit to the spec at the moment.

{x: null}.x
// null

{}.x  
// null

Note that the FEEL engine behaves currently differently if the variable/property doesn't exist. It just fails but this is a subject of change (see the discussion here). Additionally, the is defined() function is not part of the DMN spec.


One alternative solution for the use case with the REST API could be a filter on the context.

For example, filtering all null values:

context(
  get entries(
    {"a": 1, "b": null, "c": 3}
  )[value != null]
)
// {"c":3,"a":1}

Or, filtering by selected keys:

context(
  get entries(
    {"a": 1, "b": 2, "c": 3}
  )[key in ["a", "c"])
)
// {"c":3,"a":1}

With the current behavior, you may need to use is define() for optional variables/values.

context(
  get entries({
    "a": if defined(a) then a else null, 
    "b": if defined(b) then b else null, 
    "c": if defined(c) then c else null}
  )[value != null]
)

This context expression would look better with the planned get or else() function.

@korthout do you think that the alternative would work for the use case?

@saig0 saig0 removed their assignment May 16, 2023
@korthout
Copy link
Member Author

Thanks @saig0 That's an interesting alternative 👏

I think it works for most situations, but things get tricky when both null and missing properties need to be combined. For example, using PUT to set a specific property to null while ignoring the other properties.

Having said that, I think this alternative is better than what I came up with. So thank you.

I'm fine with closing this for now, as it's unlikely we'll support the undefined keyword.

@korthout korthout closed this as not planned Won't fix, can't repro, duplicate, stale May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants