camunda/feel-scala

Recasting "date and time" results in null

Closed this issue · 7 comments

abbasadel commented

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 
<?xml version="1.0" encoding="UTF-8"?>
<definitions xmlns="https://www.omg.org/spec/DMN/20191111/MODEL/" xmlns:dmndi="https://www.omg.org/spec/DMN/20191111/DMNDI/" xmlns:dc="http://www.omg.org/spec/DMN/20180521/DC/" xmlns:modeler="http://camunda.org/schema/modeler/1.0" id="Definitions_1dyiz3a" name="DRD" namespace="http://camunda.org/schema/1.0/dmn" exporter="Camunda Modeler" exporterVersion="5.12.0" modeler:executionPlatform="Camunda Cloud" modeler:executionPlatformVersion="8.2.0">
  <decision id="Decision_0kdaoiu" name="Decision 1">
    <variable id="InformationItem_1ely7et" name="test" />
    <literalExpression id="LiteralExpression_08ot0qg">
      <text>

{   builtin_now: now(),   builtin_now_again: date and time(builtin_now) }
</text>
    </literalExpression>
  </decision>
  <dmndi:DMNDI>
    <dmndi:DMNDiagram>
      <dmndi:DMNShape id="DMNShape_0u9e1z6" dmnElementRef="Decision_0kdaoiu">
        <dc:Bounds height="80" width="180" x="160" y="100" />
      </dmndi:DMNShape>
    </dmndi:DMNDiagram>
  </dmndi:DMNDI>
</definitions>

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

  • FEEL engine version: 1.16.0

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

korthout commented

ZPA triage:

  • severity: unable to use conversion function from date and time to date and time
  • bug seems reasonable to resolve
  • expected size: small
  • marking as later
  • might be interesting as a mob programming topic
nicpuppa commented

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.

  • 👍2
abbasadel commented

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.

  • 👍1
  • 👀1
abbasadel commented

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

  • 👍1
saig0 commented

@abbasadel I tend to reject this feature request.

Reasoning:

  • I see no relevant use case for extending the built-in function date and time() to accept a date-and-time value and return it as it is. If there is a use case, please share it to demonstrate the need.
  • Without a good use case, it is unlike that the DMN spec will adopt this change. As a result, this function will end up as a Camunda extension. We prefer to stay close to the DMN spec.
  • There are no other built-in functions that behave like the expected behavior of this request. We should aim for a consistent behavior of all built-in functions to keep FEEL "friendly" (i.e. easy to use).

I recommend rethinking how the expressions are modeled.

  • 👍1
abbasadel commented

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

  • 👍1
saig0 commented

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

  • 👍1