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

[VL] Wrong result when cast(double as decimal) with gluten code: branch-1.1 #4947

Closed
kecookier opened this issue Mar 13, 2024 · 6 comments
Closed
Labels
bug Something isn't working triage

Comments

@kecookier
Copy link
Contributor

Backend

VL (Velox)

Bug description

The following unittest can reproduce this issue.
I test this in branch-1.1, and not sure if this issue still exists in the branch-1.1.1.

TEST_F(CastExprTest, myTestDouble2Decimal) {
  testComplexCast(
      "c0",
      makeFlatVector<double>({0.0, 0.575}),
      makeFlatVector<int128_t>({0, 58}, DECIMAL(20, 2)));
}

Spark version

None

Spark configurations

No response

System information

No response

Relevant logs

/root/gluten/ep/build-velox/build/velox_ep/velox/vector/tests/utils/VectorTestBase.cpp:151: Failure
Value of: expected->equalValueAt(actual.get(), i, i)
  Actual: false
Expected: true
at 1: expected 0.58, but got 0.57
@kecookier kecookier added bug Something isn't working triage labels Mar 13, 2024
@kecookier
Copy link
Contributor Author

hi @rui-mo, Can you help fix this issue?

@rui-mo
Copy link
Contributor

rui-mo commented Mar 13, 2024

@kecookier OK, I'll take a look.

@rui-mo
Copy link
Contributor

rui-mo commented Mar 13, 2024

@kecookier This issue does not exist on the main branch, as we recently merged a fix to the precision issue of cast(double as decimal). Just verified below test on main.

TEST_F(CastExprTest, myTestDouble2Decimal) {
  testCast(
      makeFlatVector<double>({0.0, 0.575}),
      makeFlatVector<int128_t>({0, 58}, DECIMAL(20, 2)));
}

@kecookier
Copy link
Contributor Author

@rui-mo Could you provide the PR for the fix? I'd like to cherry-pick it for testing.

@rui-mo
Copy link
Contributor

rui-mo commented Mar 13, 2024

@kecookier Please take a try with facebookincubator/velox#8575.

@kecookier
Copy link
Contributor Author

@rui-mo I've test this PR in release-1.1, it has fixed the issue.
Thanks for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage
Projects
None yet
Development

No branches or pull requests

2 participants