Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI][C++] TestDecimalFromReal for Float type failed in "AMD64 Windows MinGW MINGW32 C++" #35606

Closed
mapleFU opened this issue May 16, 2023 · 11 comments · Fixed by #35680
Closed

Comments

@mapleFU
Copy link
Member

mapleFU commented May 16, 2023

Describe the bug, including details regarding any error messages, version, and platform.

The error messages is listed below:

[----------] 2 tests from TestDecimalFromReal/1, where TypeParam = std::pair<arrow::Decimal128, double>
[ RUN      ] TestDecimalFromReal/1.TestSuccess
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "4611686000000000000"
  expected
    Which is: "4611686018427387904"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-4611686000000000000"
  expected
    Which is: "-4611686018427387904"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "9223372000000000000"
  expected
    Which is: "9223372036854775808"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-9223372000000000000"
  expected
    Which is: "-9223372036854775808"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "18446744000000000000"
  expected
    Which is: "18446744073709551616"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-18446744000000000000"
  expected
    Which is: "-18446744073709551616"
[  FAILED  ] TestDecimalFromReal/1.TestSuccess, where TypeParam = std::pair<arrow::Decimal128, double> (0 ms)
[ RUN      ] TestDecimalFromReal/1.TestErrors
[       OK ] TestDecimalFromReal/1.TestErrors (0 ms)
[----------] 2 tests from TestDecimalFromReal/1 (0 ms total)

[----------] 2 tests from TestDecimalFromReal/2, where TypeParam = std::pair<arrow::Decimal256, float>
[ RUN      ] TestDecimalFromReal/2.TestSuccess
[       OK ] TestDecimalFromReal/2.TestSuccess (0 ms)
[ RUN      ] TestDecimalFromReal/2.TestErrors
[       OK ] TestDecimalFromReal/2.TestErrors (0 ms)
[----------] 2 tests from TestDecimalFromReal/2 (0 ms total)

[----------] 2 tests from TestDecimalFromReal/3, where TypeParam = std::pair<arrow::Decimal256, double>
[ RUN      ] TestDecimalFromReal/3.TestSuccess
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "4611686000000000000"
  expected
    Which is: "4611686018427387904"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-4611686000000000000"
  expected
    Which is: "-4611686018427387904"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "9223372000000000000"
  expected
    Which is: "9223372036854775808"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-9223372000000000000"
  expected
    Which is: "-9223372036854775808"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "18446744000000000000"
  expected
    Which is: "18446744073709551616"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-18446744000000000000"
  expected
    Which is: "-18446744073709551616"
[  FAILED  ] TestDecimalFromReal/3.TestSuccess, where TypeParam = std::pair<arrow::Decimal256, double> (0 ms)
[ RUN      ] TestDecimalFromReal/3.TestErrors

(1) (3) in TestDecimalFromReal are all float. So I guess there would be something wrong when handling float type. The CI can be seen here: https://github.com/apache/arrow/actions/runs/4988181428/jobs/8930665332?pr=35605

Component(s)

C++, Continuous Integration

@mapleFU
Copy link
Member Author

mapleFU commented May 16, 2023

@pitrou Mind take a look?

@pitrou
Copy link
Member

pitrou commented May 16, 2023

These tests haven't changed in a while AFAIK, and they used to pass on MinGW. If possible it would be nice to research the GHA history and find out at which point they started failing.

@pitrou
Copy link
Member

pitrou commented May 16, 2023

Also cc @kou since it's MinGW-related.

@mapleFU
Copy link
Member Author

mapleFU commented May 16, 2023

Yes, #35571 is also MinGW-related. I guess they have similiar reason. I submit a hotfix but I guess maybe we'd better find the root cause

@pitrou
Copy link
Member

pitrou commented May 16, 2023

Where is your hotfix?

@mapleFU
Copy link
Member Author

mapleFU commented May 16, 2023

( #35605 . Just for float point and parquet decryption, not for this issue )

@kou
Copy link
Member

kou commented May 16, 2023

I tried this on my MSYS2 environment but this is not reproduced...
Anyway, I'll take a look at this.

@westonpace
Copy link
Member

Is this closed by #35605 / #35571 ?

@mapleFU
Copy link
Member Author

mapleFU commented May 18, 2023

Is this closed by #35605 / #35571 ?

@westonpace I think they're different problems

@mapleFU
Copy link
Member Author

mapleFU commented May 18, 2023

// Common tests for Decimal128::FromReal(T, ...) and Decimal256::FromReal(T, ...)
template <typename T>
class TestDecimalFromReal : public ::testing::Test {
 public:
  using Decimal = typename T::first_type;
  using Real = typename T::second_type;
  using ParamType = FromRealTestParam<Real>;

  void TestSuccess() {
    const std::vector<ParamType> params{
        // clang-format off
        {0.0f, 1, 0, "0"},
        {-0.0f, 1, 0, "0"},
        {0.0f, 19, 4, "0.0000"},
        {-0.0f, 19, 4, "0.0000"},
        {123.0f, 7, 4, "123.0000"},
        {-123.0f, 7, 4, "-123.0000"},
        {456.78f, 7, 4, "456.7800"},
        {-456.78f, 7, 4, "-456.7800"},
        {456.784f, 5, 2, "456.78"},
        {-456.784f, 5, 2, "-456.78"},
        {456.786f, 5, 2, "456.79"},
        {-456.786f, 5, 2, "-456.79"},
        {999.99f, 5, 2, "999.99"},
        {-999.99f, 5, 2, "-999.99"},
        {123.0f, 19, 0, "123"},
        {-123.0f, 19, 0, "-123"},
        {123.4f, 19, 0, "123"},
        {-123.4f, 19, 0, "-123"},
        {123.6f, 19, 0, "124"},
        {-123.6f, 19, 0, "-124"},
        // 2**62
        {4.611686e+18f, 19, 0, "4611686018427387904"},
        {-4.611686e+18f, 19, 0, "-4611686018427387904"},
        // 2**63
        {9.223372e+18f, 19, 0, "9223372036854775808"},
        {-9.223372e+18f, 19, 0, "-9223372036854775808"},
        // 2**64
        {1.8446744e+19f, 20, 0, "18446744073709551616"},
        {-1.8446744e+19f, 20, 0, "-18446744073709551616"}
        // clang-format on
    };
    for (const ParamType& param : params) {
      CheckDecimalFromReal<Decimal>(param.real, param.precision, param.scale,
                                    param.expected);
    }
  }

Here meets:

        // 2**62
        {4.611686e+18f, 19, 0, "4611686018427387904"},
        {-4.611686e+18f, 19, 0, "-4611686018427387904"},
        // 2**63
        {9.223372e+18f, 19, 0, "9223372036854775808"},
        {-9.223372e+18f, 19, 0, "-9223372036854775808"},
        // 2**64
        {1.8446744e+19f, 20, 0, "18446744073709551616"},
        {-1.8446744e+19f, 20, 0, "-18446744073709551616"}

These values would be truncated on MinGW when type is float

@kou
Copy link
Member

kou commented May 19, 2023

I could reproduce this.

kou added a commit to kou/arrow that referenced this issue May 19, 2023
Large/small double values are truncated with MinGW 32bit.
kou added a commit to kou/arrow that referenced this issue May 19, 2023
Large/small double values are truncated with MinGW 32bit.
@pitrou pitrou added this to the 13.0.0 milestone May 19, 2023
pitrou pushed a commit that referenced this issue May 19, 2023
…al test (#35680)

### Rationale for this change

If we use `4.611686e+18f` for `2**62` value, MinGW 32bit can't generate enough precious double value.
See the following errors for this:

```text
[----------] 2 tests from TestDecimalFromReal/1, where TypeParam = std::pair<arrow::Decimal128, double>
[ RUN      ] TestDecimalFromReal/1.TestSuccess
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "4611686000000000000"
  expected
    Which is: "4611686018427387904"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-4611686000000000000"
  expected
    Which is: "-4611686018427387904"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "9223372000000000000"
  expected
    Which is: "9223372036854775808"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-9223372000000000000"
  expected
    Which is: "-9223372036854775808"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "18446744000000000000"
  expected
    Which is: "18446744073709551616"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-18446744000000000000"
  expected
    Which is: "-18446744073709551616"
[  FAILED  ] TestDecimalFromReal/1.TestSuccess, where TypeParam = std::pair<arrow::Decimal128, double> (0 ms)
[ RUN      ] TestDecimalFromReal/1.TestErrors
[       OK ] TestDecimalFromReal/1.TestErrors (0 ms)
[----------] 2 tests from TestDecimalFromReal/1 (0 ms total)

[----------] 2 tests from TestDecimalFromReal/2, where TypeParam = std::pair<arrow::Decimal256, float>
[ RUN      ] TestDecimalFromReal/2.TestSuccess
[       OK ] TestDecimalFromReal/2.TestSuccess (0 ms)
[ RUN      ] TestDecimalFromReal/2.TestErrors
[       OK ] TestDecimalFromReal/2.TestErrors (0 ms)
[----------] 2 tests from TestDecimalFromReal/2 (0 ms total)

[----------] 2 tests from TestDecimalFromReal/3, where TypeParam = std::pair<arrow::Decimal256, double>
[ RUN      ] TestDecimalFromReal/3.TestSuccess
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "4611686000000000000"
  expected
    Which is: "4611686018427387904"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-4611686000000000000"
  expected
    Which is: "-4611686018427387904"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "9223372000000000000"
  expected
    Which is: "9223372036854775808"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-9223372000000000000"
  expected
    Which is: "-9223372036854775808"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "18446744000000000000"
  expected
    Which is: "18446744073709551616"
D:/a/arrow/arrow/cpp/src/arrow/util/decimal_test.cc:766: Failure
Expected equality of these values:
  dec.ToString(scale)
    Which is: "-18446744000000000000"
  expected
    Which is: "-18446744073709551616"
[  FAILED  ] TestDecimalFromReal/3.TestSuccess, where TypeParam = std::pair<arrow::Decimal256, double> (0 ms)
[ RUN      ] TestDecimalFromReal/3.TestErrors
```

### What changes are included in this PR?

This changes use `4.6116860184273879e+18` for `2**62` value. It works with MinGW 32bit too.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* Closes: #35606

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants