Interpreter erroneously uses unsigned integers

Problem The tremor-runtime interpreter implements binary operators to, e.g., divide two numbers or concatenate two strings. When both arguments to a binary operator are positive integers which fall within the range of 64 bit unsigned integers, the interpreter converts both arguments to native u64s, and works with that. This behavior is however not sound for subtraction.

For instance, while 1 and 2 are definitely valid u64s, an expression 1 - 2 should evaluate to -1. Native u64s instead panic on debug builds, and wrap to u64::max_value() on release builds.

Steps In general, feeding an expression like 2 - 1; into the tremor-script interpreter will trigger this behavior.

To reproduce this programmatically, consider adding this test case to the tests in tremor-script/src/lib.rs:

#[test]
fn test_arith_expr() {
    eval!("1 + 1;", Value::from(2));
    eval!("2 - 1;", Value::from(1));
    eval!("1 - 2;", Value::from(-1));
}

Possible Solution(s) The main culprit is this line, where subtraction of tremor values is handled by subtracting native u64s. A very local hotfix could affect only that line. In general, though, I think it's important to nail down the expected overflow behavior of tremor-script.

Perhaps the cleanest solution, is to remove the conversion to u64 entirely for arithmetic operations, handling all integers as i64s (or even handling all numbers as f64s?). Of course, in that case 9223372036854775807 + 1 would overflow by exhausting i64, even though a u64 could handle this just fine. On the other hand, code that handles the data as double precision floating points (i.e., JSON and JavaScript, among others) would be unable to represent this as proper integers way sooner. Furthermore, a similar hack is already in place two lines below it to handle division.

Notes

~Output of rustup --version:~ ~Output of rustup show:~ ~Output of tremor-server --version:~ This issue relates to the code in tremor-runtime/tremor-script/src/interpreter.rs as of Git commit hash 10a877781e1f3c4e230aaa1fb55dddb29238627f

tremor-rs / tremor-runtime

Interpreter erroneously uses unsigned integers #134