dmn-tck / tck

Decision Model and Notation Technology Compatibility Kit
https://dmn-tck.github.io/tck
51 stars 36 forks source link

Expected behavior for list built-in functions in combination with singleton lists? #73

Closed DanielThanner closed 6 years ago

DanielThanner commented 7 years ago

The behavior of singleton lists is not clearly specified in the DMN specification. Especially for list built-in functions there are some special cases:

operation expected result 1 expected result 2 expected result 3
distinct values(["a", ["a"]]) ["a"] ["a", ["a"]] [["a"]]
union(["a"], [["a"]]) ["a"] ["a", ["a"]]
index of(["a", ["a"]], "a") [1,2] [1]
count([[]]) 0 1
list contains(["a", ["b"]], "b") true false

Should the elements of the lists be normalized (transform singleton lists to single element) before the operation is applied? If this is the case column "expected result 1" shows the return values.

Should the elements treated "as is", without any transformation. It this is the case column "expected result 2" shows the return values.

For distinct values() there is also a valid 3rd result (as alternative to expected result 1 if normalization was done). Should we return element 1 or 2 of the list. In this case element 2 is returned.

Which results do you calculate/expect in your engine/runtime? Which is the valid approach?

DanielThanner commented 7 years ago

Chapter 10.3.1.4 in the spec on page 110: "A singleton list is equal to its single item, i.e., [e] = e for all FEEL expressions e."

arthware commented 7 years ago

@opatrascoiu @agilepro @etirelli @falko

Any thoughts / opinions / emotions on this? From my point of view: If DMN/FEEL implementors do not agree on a consistent behavior for this, cross-runtime compatibility is theoretical for DMN. This is a crucial behavior to discuss. The specification would allow both results - with potentially drastical impact on model behavior.

brsilver commented 7 years ago

The spec is clear. As you quote: Chapter 10.3.1.4 in the spec on page 110: "A singleton list is equal to its single item, i.e., [e] = e for all FEEL expressions e."

To me that implies, from your list…

  1. Distinct values – all 3 are correct and equal to “a”

  2. Union – Both are correct and equal to “a”. Also count(union([“a”],[[“a”]]))=1

  3. Index of([“a”, [“a”]],”a”) = [1,2]

  4. Count([[“a”]])=1. Clearly this is not a 0-element list.

  5. List contains([“a”,[“b”]],”b”) = true, although this one seems the most ambiguous.

Note these problematic examples come from lists of mixed type, which have no possible item definition (although they are not specifically disallowed).

etirelli commented 7 years ago

I agree with @arthware, that unless the semantics for this is clearly defined, it breaks interoperability between products.

Unfortunately I don't think there is a "solution" for this in DMN 1.1, because the "[a] = a" makes the whole semantics completely ambiguous.

There is also, to my knowledge, no proposal yet to fix this for DMN 1.2. So the only solution would be for us to make a proposal of what the semantics should be.

This might actually become worse in DMN 1.2 because there is a proposal to support heterogenous lists in DMN 1.2, while DMN 1.1 if I remember correctly only supports homogeneous lists.

So, I believe we could focus here on a proposal to submit for DMN 1.2, as the discussion of any function in particular would be a moot point. In particular, I believe the solution would be to constrain the "[a] = a" coercion to only the actual places where it would be applicable/safe.

arthware commented 7 years ago

Some FEEL puzzles:

@brsilver The result type is relevant if results of list functions are nested. Therefore, allowing all 3 results in the first example could end up in different runtime behaviors once invocation results are nested (I believe).

The count([[“a”]])=1 example was wrong in the original post. We meant count([[]])

brsilver commented 7 years ago

Based on this thread, probably important to fix this in DMN1.2. I think the origin of the problem was basing FEEL on XPath although XPath does not have nested lists. From a practical standpoint, the most important place I would like to retain [a]=a is for filter expressions, particularly when they select on a unique key, which is very common. In that case calling the result a list is counter-intuitive. I think a detailed proposal (e.g. what do you mean by applicable/safe?) would probably pass in RTF.

etirelli commented 7 years ago

@arthware yeah, there are plenty of problems like the ones you mentioned. There is no answer in DMN 1.1.

Would you like to try and come up with a proposal for DMN 1.2? I can help get it through the RTF process.

DanielThanner commented 7 years ago

@brsilver @etirelli

I have created a new ticket at the OMG with a proposal for DMN 1.2. It is not a change of the spec, but a clarification of the expected behavior. If the OMG solution for DMN 1.2 is compatible with 1.1 we can choose this way in DMN TCK, too (unless 1.2 is released).

(http://issues.omg.org/browse/INBOX-556)

brsilver commented 7 years ago

Daniel,

This is great. In addition to the built-in functions, there is the issue of filter expressions. For example,

MyTable[id=”abc”].x = y

If the filter returns a singleton list, I would rather have this than

MyTable[id=”abc”].x[1] = y

Can your rules resolve this one?

etirelli commented 7 years ago

@DanielThanner thank you! as soon as the issue pass the triage, we can bring it up on the RTF meeting.

DanielThanner commented 7 years ago

@brsilver The specification says for filters: "... and a list containing only items where the filter expression is true is returned.". This means a filter always returns a list. In case only a single element is returned, it is inside a list, a singleton list.

You can use [1] to access the first element. But I don't think that this is necessary, because the spec says that "e = [e] for all FEEL expressions e". Therefore, if you are sure that the filter matches a single element, you will get a singleton list as result. You can pass the result of the filter to any FEEL expression. It doesn't makes a difference if you work with the singleton list [e] or the singleton e itself (some exceptions for built-in functions list contains(), index of() and count()).

brsilver commented 7 years ago

This works for me, but I remember Edson saying it was possible only under certain circumstances.

--Bruce

etirelli commented 7 years ago

@brsilver just to clarify, what I said is that the conversion sometimes is ambiguous, specially when you compose a filter into another filter or as a parameter to a function, very similar to the examples raised by @DanielThanner and @arthware .

For instance:

append( someList, MyTable[id=”abc”].x )

Lets say the filter in the example above returns ["foo"] . Are you appending ["foo"] to someList or are you appending "foo" ?

What if, because of a problem in the MyTable data, the result for x is ["foo","bar"], but someList is a list of Strings. Now it is an invalid list and in the best case raises an error at runtime. Worst case, silently fails.

brsilver commented 7 years ago

Understood. But it would be best if proposed clarification disambiguates this use case, in addition to list functions.

From: Edson Tirelli [mailto:notifications@github.com] Sent: Thursday, October 19, 2017 10:00 AM To: dmn-tck/tck Cc: brsilver; Mention Subject: Re: [dmn-tck/tck] Expected behavior for list built-in functions in combination with singleton lists? (#73)

@brsilver https://github.com/brsilver just to clarify, what I said is that the conversion sometimes is ambiguous, specially when you compose a filter into another filter or as a parameter to a function, very similar to the examples raised by @DanielThanner https://github.com/danielthanner and @arthware https://github.com/arthware .

For instance:

append( someList, MyTable[id=”abc”].x )

Lets say the filter in the example above returns ["foo"] . Are you appending ["foo"] to someList or are you appending "foo" ?

What if, because of a problem in the MyTable data, the result for x is ["foo","bar"], but someList is a list of Strings. Now it is an invalid list and in the best case raises an error at runtime. Worst case, silently fails.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dmn-tck/tck/issues/73#issuecomment-337971683, or mute the thread https://github.com/notifications/unsubscribe-auth/AL-HkB_KW7i8JKNCLGdJR6MzBe1WOQcSks5st3_8gaJpZM4P8Hw5 .[image: Image removed by sender.]

DanielThanner commented 7 years ago

For the filter topic there is already a JIRA issue at OMG: http://issues.omg.org/browse/DMN12-115

The text in the spec does not match the FEEL example in the spec.

brsilver commented 7 years ago

Yes there is RTF issue on it, but I think up to TCK to resolve it.

From: DanielThanner [mailto:notifications@github.com] Sent: Thursday, October 19, 2017 9:35 PM To: dmn-tck/tck Cc: brsilver; Mention Subject: Re: [dmn-tck/tck] Expected behavior for list built-in functions in combination with singleton lists? (#73)

For the filter topic there is already a JIRA issue at OMG: http://issues.omg.org/browse/DMN12-115

The text in the spec does not match the FEEL example in the spec.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dmn-tck/tck/issues/73#issuecomment-338103762, or mute the thread https://github.com/notifications/unsubscribe-auth/AL-HkP1J6zDFWP4puMa0XSFlbz9LXVPfks5suCMEgaJpZM4P8Hw5 .[image: Image removed by sender.]

falko commented 7 years ago

At Camunda we agree with Edson and would like to limit the rule or clearly define when it applies. Ideally, we would like to remove the rule to prevent ambiguities completely. But that is probably not up for debate.

DanielThanner commented 7 years ago

@etirelli Can you please add example(s) of nested filters that have problems with singleton lists?

opatrascoiu commented 6 years ago

Reminder RTF ticket is http://issues.omg.org/browse/DMN12-115

etirelli commented 6 years ago

Linking the ticket with the proposal: https://issues.omg.org/browse/DMN12-210

agilepro commented 6 years ago

Approved by RTF. Nothing we can do in short term. When this gets implemented, we might run into problems that need addressing. We probably will need a set of tests on this.