VRL function fallibility is dependant on argument types

[jorgebay]

A note for the community

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Problem

The built-in functions to coerce values can be both fallible and not fallible depending on the value.

The functions are documented as fallible but if the compiler can detect that the value is of a certain type, it changes the signature from fallible to not fallible.

For example, this is a valid syntax and compiles:

str = "something"
result, err = to_bool(str)

But this fails:

b = false
result, err = to_bool(b)

with

error[E104]: unnecessary error assignment
  ┌─ :1:9
  │
1 │ result, err = to_bool(b)
  │ ------  ^^^   ---------- because this expression can't fail
  │ │       │
  │ │       this error assignment is unnecessary
  │ use: result = to_bool(b)

The same occurs with to_float(), to_int(), ...

The signature changes from:

to_bool(value: <boolean | integer | float | null | string>)
:: <boolean> , <error>

to:

to_bool(value: <boolean>) :: <boolean> // not fallible

As a language user this can be confusing, a function should not change fallibility depending on the parameter value.

Configuration

No response

Version

0.27

Debug Output

No response

Example Data

No response

Additional Context

No response

References

No response

[jszwedko]

Hey @jorgebay !

Thanks for opening this!

As a language user this can be confusing, a function should not change fallibility depending on the parameter value.

I realize it can be a bit confusing, but that is actually a core part of the language. Function fallibility is conditional on whether the function knows that the types of the incoming arguments are compatible. If it doesn't know, then the function becomes fallible even if it is infallible when the argument types are known to be compatible.

This is good feedback, but changing it would mean changing some of the core principles of VRL so it is unlikely to happen in the near future. I'll leave this issue open to collect additional user feedback.

[jorgebay]

From the user pov, I think it makes more sense to have something like

to_bool(value: any) :: <boolean>, <error>

or

to_bool(value: <integer | float | null | string>) :: <boolean>, <error>  // no bool

Than a signature specific to the target type that is a no-op.

Currently, from the signature perspective, It's 2 different functions:

to_bool(value: <integer | float | null | string>) :: <boolean>, <error>  // no bool
to_bool(value: <boolean>) :: <boolean>  // noop

If the decision is to continue using these signatures, we can update the documentation to reflect dual signature / function overloading.

Coming from golang/rust, function overloading feels strange to me.

[jszwedko]

Nice, I like that idea of providing multiple definitions depending on arguments. I agree that the overloading is a bit wonky. The idea of making functions fallible when the argument types aren't known to be correct was intended to be a usability improvement vs. requiring users to explicitly cast each time to narrow types, but it has been some source of confusion and so could bear revisiting in the future.

[dekelpilli]

I like this idea. It's worth noting that if this was implemented (such that v, err = to_bool(true) is a valid piece of VRL code), #91 would no longer be a breaking change.

[XC-]

Hey @jorgebay !

Thanks for opening this!

As a language user this can be confusing, a function should not change fallibility depending on the parameter value.

I realize it can be a bit confusing, but that is actually a core part of the language. Function fallibility is conditional on whether the function knows that the types of the incoming arguments are compatible. If it doesn't know, then the function becomes fallible even if it is infallible when the argument types are known to be compatible.

This is good feedback, but changing it would mean changing some of the core principles of VRL so it is unlikely to happen in the near future. I'll leave this issue open to collect additional user feedback.

@jszwedko

Is this mentioned somewhere in the documentation? If it is, I cannot find where.

Also, I don't know if this is related. But when I use vector vrl with input file and program file, I see quite different behavior than when I define the input ( . = something) in the REPL mode. It is claiming that several functions that the documentation says are not fallible, suddenly are fallible. Now, if I add an exclamation mark to the function (e.g. replace!(..., then it says the functions are not fallible.

I am using vector 0.32.2.

This kind of behavior is making development with the REPL mode kinda difficult.

[jorgebay]

@XC- The issue you are referring to doesn’t seem related to the functions listed in this ticket.

Note that assignment expressions can be fallible in VRL language: https://vector.dev/docs/reference/vrl/expressions/#assignment

[XC-]

@jorgebay I disagree with the assignment being fallible. Nothing in the assignment documentation states that it could be fallible but that there can be a second argument which is the error from a fallible right side expression.

error argument meaning in the documentation:

The error allows for optional assignment to errors when the right-hand-side expression is fallible. This is commonly used when invoking fallible functions.

Of course, it can always be that the documentation is lacking, but until someone verifies it to be otherwise, I'd say that the assignment is not fallible (by design at least).

[jszwedko]

#560 improves the error messaging for this. Just noting again that the fallibility depending on argument types is a fundamental part of VRL so it is unlikely to change in the near future, but could be something that changes if we went to a VRL 2.0.