terrajobst / nquery

NQuery is a relational query engine written in C#. It allows you to execute a SELECT query against .NET objects.
MIT License
72 stars 20 forks source link

Query generates incorrect result when using constants, works fine with parameters #32

Closed dallmair closed 8 years ago

dallmair commented 8 years ago

Run the following query in NQuery:

SELECT ROUND(TO_DECIMAL(1.23), 4) AS Col1, TO_DECIMAL(TO_STRING(1.23) + '00') AS Col2

Actual result is a table with a single row and two columns, Col1 and Col2. Both have the value 1.23. Expected result is a table in the same format, but with values 1.23 and 1.2300, respectively.

Interesting facts:

  1. Change the first 1.23 to 1.24 and NQuery generates the expected result 1.24, 1.2300.
  2. Provide the value 1.23 as parameter instead of coding it as constant. In this case NQuery also generates the expected result 1.23, 1.2300.
terrajobst commented 8 years ago

That's an interesting case. First I thought it's a bug in the constant folding but it's actually by-design. You could argue that it's unfortunate, but it's based on the behavior of decimal.Equals. I've rewritten the code below to bypass the constant folding. As you can see, the problem is that 1.23m and 1.2300m are considered equal, which is why the constant folder collapsed them into the same value slot.

WITH Value AS (
    SELECT 1.23 AS Value
),
RoundedValues AS (
    SELECT ROUND(TO_DECIMAL(Value), 2) TwoDigits,
           ROUND(TO_DECIMAL(TO_STRING(Value) + '00'), 4) FourDigits
    FROM   Value v
)
SELECT  r.TwoDigits,               -- 1.23
        r.FourDigits,              -- 1.2300
        r.TwoDigits = r.FourDigits -- True
FROM    RoundedValues r

You can replicate the behavior in C#:

const decimal twoDigits = 1.23m;
const decimal fourDigits = 1.2300m;
const bool equals = twoDigits == fourDigits;

Console.WriteLine(twoDigits);               // 1.23
Console.WriteLine(fourDigits);              // 1.2300
Console.WriteLine(twoDigits == fourDigits); // true
Console.WriteLine(equals);                  // true
terrajobst commented 8 years ago

By the way, this behavior doesn't repro in nquery-vnext because constant folding hasn't been implemented yet. I'm wondering whether adding constant folding is actually a good idea; the value seems to be quite low but it complicates the design of the semantic model quite a bit. Right now I'm thinking that nquery-vnext will not perform constant folding.

dallmair commented 8 years ago

Thanks for taking the time to analyze the issue. Fully agree with your conclusion, because 1.23 is in fact equal to 1.2300 (though not the same thing). Regarding constant folding I also think along the same lines as it seems to cause some other problems as well (e.g. #7).

terrajobst commented 8 years ago

Fair, although #7 is arguably simply a bug. I'm more concerned with issues like this where the behavior is unfortunate but correct :smile: