Skip to content

Commit

Permalink
fix(vrl)!: Make remainder fallible (vectordotdev#11668)
Browse files Browse the repository at this point in the history
* Make remainder fallible.

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>

* Update release notes.

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>

* Added test for remainder by zero

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>

* Newline

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>

* Whitespace

Signed-off-by: Stephen Wakely <fungus.humungus@gmail.com>
  • Loading branch information
StephenWakely authored and pull[bot] committed May 11, 2023
1 parent b563c40 commit 755da8c
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 4 deletions.
27 changes: 23 additions & 4 deletions lib/vrl/compiler/src/expression/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,21 @@ impl Expression for Op {
}
}

// ... % ...
Rem => {
// Division is infallible if the rhs is a literal normal float or integer.
match self.rhs.as_value() {
Some(value) if lhs_def.is_float() || lhs_def.is_integer() => match value {
Value::Float(v) if v.is_normal() => TypeDef::float().infallible(),
Value::Float(_) => TypeDef::float().fallible(),
Value::Integer(v) if v != 0 => TypeDef::integer().infallible(),
Value::Integer(_) => TypeDef::integer().fallible(),
_ => TypeDef::float().add_integer().fallible(),
},
_ => TypeDef::float().add_integer().fallible(),
}
}

// "bar" + ...
// ... + "bar"
Add if lhs_def.is_bytes() || rhs_def.is_bytes() => lhs_def
Expand All @@ -216,7 +231,7 @@ impl Expression for Op {
// 1.0 - ...
// 1.0 * ...
// 1.0 % ...
Add | Sub | Mul | Rem if lhs_def.is_float() || rhs_def.is_float() => lhs_def
Add | Sub | Mul if lhs_def.is_float() || rhs_def.is_float() => lhs_def
.fallible_unless(K::integer().or_float())
.merge_deep(rhs_def.fallible_unless(K::integer().or_float()))
.with_kind(K::float()),
Expand All @@ -225,7 +240,7 @@ impl Expression for Op {
// 1 - 1
// 1 * 1
// 1 % 1
Add | Sub | Mul | Rem if lhs_def.is_integer() && rhs_def.is_integer() => {
Add | Sub | Mul if lhs_def.is_integer() && rhs_def.is_integer() => {
lhs_def.merge_deep(rhs_def).with_kind(K::integer())
}

Expand All @@ -247,8 +262,7 @@ impl Expression for Op {
.with_kind(K::bytes().or_integer().or_float()),

// ... - ...
// ... % ...
Sub | Rem => lhs_def
Sub => lhs_def
.merge_deep(rhs_def)
.fallible()
.with_kind(K::integer().or_float()),
Expand Down Expand Up @@ -579,6 +593,11 @@ mod tests {
want: TypeDef::integer().infallible(),
}

remainder_integer_zero {
expr: |_| op(Rem, 5, 0),
want: TypeDef::integer().fallible(),
}

remainder_float {
expr: |_| op(Rem, 5.0, 5.0),
want: TypeDef::float().infallible(),
Expand Down
6 changes: 6 additions & 0 deletions lib/vrl/compiler/src/value/arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,12 @@ impl VrlValueArithmetic for Value {
fn try_rem(self, rhs: Self) -> Result<Self, Error> {
let err = || Error::Rem(self.kind(), rhs.kind());

let rhv = rhs.try_into_f64().map_err(|_| err())?;

if rhv == 0.0 {
return Err(Error::DivideByZero);
}

let value = match self {
Value::Integer(lhv) if rhs.is_float() => {
Value::from_f64_or_zero(lhv as f64 % rhs.try_float()?)
Expand Down
4 changes: 4 additions & 0 deletions lib/vrl/tests/tests/expressions/arithmetic/remainder/zero.vrl
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# result: 0
# object: { "zero": 0 }

x = 43 % int!(.zero) ?? 0
19 changes: 19 additions & 0 deletions website/content/en/highlights/2022-03-22-0-21-0-upgrade-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Vector's 0.21.0 release includes **breaking changes**:
3. [Deprecated GraphQL API subscriptions have been removed](#removed-deprecated-subscriptions)
4. [The `vector vrl` timezone flag `-tz` is now `-z`](#vector-vrl-timezone)
5. [The `vector top` human_metrics flag `-h` is now `-H`](#vector-top-human-metrics)
6. [Remainder operator (%) in VRL is fallible](#remainder-fallible)

And **deprecations**:

Expand Down Expand Up @@ -151,6 +152,24 @@ v3](https://crates.io/crates/clap), the short `-h` from `--help` and `-h` from
shortened form for `--human_metrics` is now `-H` and `-h` is reserved for
`--help`.

#### Remainder operator (%) in VRL is fallible {#remainder-fallible}

The remainder operator in VRL has become a fallible operation. This is because
finding the remainder with a divisor of zero can raise an error that needs to
be handled.

Before this VRL would compile:

```coffee
.remainder = 50 % .value
```

If `.value` was zero, Vector would panic. This can be fixed by handling the error:

```coffee
.remainder = 50 % .value ?? 0
```

### Deprecations

#### `receivedEventsTotal`, `sentEventsTotal`, `sentEventsThroughput`, `receivedEventsThroughput` subscriptions have been deprecated {#deprecate-aggregate-subscriptions}
Expand Down

0 comments on commit 755da8c

Please sign in to comment.