ietf-wg-jsonpath / draft-ietf-jsonpath-base

Development of a JSONPath internet draft
https://ietf-wg-jsonpath.github.io/draft-ietf-jsonpath-base/
Other
59 stars 20 forks source link

Function return types #401

Closed timbray closed 1 year ago

timbray commented 1 year ago

I notice that the match() function extension has return type OptionalBooleanType (true, false, or Nothing)

This seems wrong to me? A regexp is provided and either there is a match or there isn't. if the argument yields nothing then match yields false. How can Nothing be returned? (Same is true for search obviously).

gregsdennis commented 1 year ago

Nothing was introduced as a return for the cases where the inputs weren't the right kind of values, possibly returned by a singular path.

timbray commented 1 year ago

Well, if they're not the right kind of values, then they don't match the regex, right? Thus false.

timbray commented 1 year ago

I mean, we could be pedantic and rename the function existsAndMatches() but that's what I thought it meant.

cabo commented 1 year ago

It is actually useful to distinguish the three cases (invalid/nonexistent argument, valid but does not match, valid but does match).

I'm currently looking at 2.5.5 and trying to make sure that we say all the right things about function-exprs, too.

timbray commented 1 year ago

Um, I disagree, and neither of us has brought forward any evidence. Let's assume the function means existsAndIsOfStringTypeAndMatchesTheRegexp()

I think that as a developer, that is dead easy to understand and use. We already have excellent predicates to test separately for existence, if that's what we care about. Also, I think the Optional* stuff is one of the thing that's making the function type system hard to understand.

cabo commented 1 year ago

So you want to map all Nothing for Boolean functions cases into false, as far as I can tell. That is indeed simpler. Not so easy with functions that return a general JSON value. No, –1 is not better than Nothing.

timbray commented 1 year ago

More specifically: Since we have explicit ways to test for existence, and we have boolean combinations, I'm perfectly comfortable with supporting a semantic of "exists and is the right type and X" for some functions. Because it handles two very common use cases very nicely:

  1. I really mean "it exists and is the right type and matches"
  2. I'm processing fixed-type messages and I know for sure it what fields exist and what their types are.

In neither of those cases do I care about the Nothing possibility, and I think eliminating might simplify some of our other problems.

gregsdennis commented 1 year ago

I think eliminating might simplify some of our other problems.

This discussion actually depends heavily on the outcome of other discussions.

If a Boolean is a comparable, then a Nothing return is valuable because it can be compared with false.

If a Boolean is a test-expr, then a Nothing return is unnecessary as it's treated as a "false" expression (the result of 1==2).

The other discussions open right now assert that a Boolean function can't do both of these and that Boolean functions (as a whole) need to be restricted to fit into one of these categories.

goessner commented 1 year ago

In neither of those cases do I care about the Nothing possibility, and I think eliminating might simplify some of our other problems.

I totally agree ... can enter discussion this evening at the earliest ...

cabo commented 1 year ago

So it seems that we'll have Nothing as the value added to the type that otherwise has JSON values, but keep logical (≠ JSON Boolean) and nodelist without such an addition. Let's see how that works out...

glyn commented 1 year ago

I support a (much) simpler type system if we can find one that works and which doesn't compromise the rest of the spec.

On Mon, 20 Feb 2023, 10:27 cabo, @.***> wrote:

So it seems that we'll have Nothing as the value added to the type that otherwise has JSON values, but keep logical (≠ JSON Boolean) and nodelist without such an addition. Let's see how that works out...

— Reply to this email directly, view it on GitHub https://github.com/ietf-wg-jsonpath/draft-ietf-jsonpath-base/issues/401#issuecomment-1436707849, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAXF2PKVAJGEFB7CNIBJGTWYNBK3ANCNFSM6AAAAAAVAQFAGA . You are receiving this because you are subscribed to this thread.Message ID: <ietf-wg-jsonpath/draft-ietf-jsonpath-base/issues/401/1436707849@ github.com>

gregsdennis commented 1 year ago

I appreciate that everyone in the past few comments has effectively come to the same conclusion, and that it's exactly what #402 represents.

I'm happy to close my other PRs if this is the agreed-upon direction.