camunda / feel-scala

FEEL parser and interpreter written in Scala
https://camunda.github.io/feel-scala/
Apache License 2.0
123 stars 50 forks source link

Recasting "date and time" results in null #697

Closed abbasadel closed 1 year ago

abbasadel commented 1 year ago

Describe the bug Evaluate the following FEEL expression

{   builtin_now: now(),   builtin_now_again: date and time(builtin_now) }

built_now_again comes out null

To Reproduce Steps to reproduce the behavior:

Expression with redundant 'date and time' casting: { builtin_now: now(), builtin_now_again: date and time(builtin_now) }

Results in: {"builtin_now":"2023-07-31T18:39:50.009723879Z[Etc/UTC]","builtin_now_again":null}

Expression without redundant casting: { builtin_now: now(), builtin_now_again: builtin_now }

Results correctly: {"builtin_now":"2023-07-31T18:42:39.022504381Z[Etc/UTC]","builtin_now_again":"2023-07-31T18:42:39.022504381Z[Etc/UTC]"}

DMN example ``` { builtin_now: now(), builtin_now_again: date and time(builtin_now) } ```

Expected behavior builtin_now_again should equal builtin_now

Workaround Avoid redundant casting

Hint Seems like there's a case missing in datetime function in ConversionBuiltinFunctions. The warning output we get is:

15:14:10.596 [main] WARN org.camunda.feel.FeelEngine – Suppressed failure: illegal arguments: List(ValDateTime(2023-07-28T15:14:10.593081-04:00[America/New_York]))

Environment

Support: https://jira.camunda.com/browse/SUPPORT-17817

korthout commented 1 year ago

ZPA triage:

nicpuppa commented 1 year ago

Is this really a bug ? Looking at the documentation the date and time function accept only string object. The behaviour of the function is correct from my perspective.

abbasadel commented 1 year ago

Very valid point @nicpuppa. I think we can safely assume this is a feature request and not a bug. @saig0 since you are the stakeholder/codeowner, kindly have a look on this feature request.

abbasadel commented 1 year ago

Moving this issue to blocked since we are waiting for stakeholders' feedback

saig0 commented 1 year ago

@abbasadel I tend to reject this feature request.

Reasoning:

I recommend rethinking how the expressions are modeled.

abbasadel commented 1 year ago

Thanks @saig0 for your feedback. What you said makes sense to me.

saig0 commented 1 year ago

It seems that we have an agreement that we don't want to add the requested function. Let's close the issue.