-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix CAST(interval day to second as varchar) #9386
Conversation
This pull request was exported from Phabricator. Differential Revision: D55796600 |
✅ Deploy Preview for meta-velox canceled.
|
Summary: CAST expression used to handle INTERVAL DAY TO SECOND values as BIGINT. This allowed invalid casts (e.g. from varchar, to double, etc.) and produced incorrect results when casting to varchar. Casting 1 second interval used to return '1000' instead of '0 00:00:01.000'. Fixes facebookincubator#9384 Differential Revision: D55796600
b34a61e
to
fd78e99
Compare
This pull request was exported from Phabricator. Differential Revision: D55796600 |
Thanks @mbasmanova. |
Summary: CAST expression used to handle INTERVAL DAY TO SECOND values as BIGINT. This allowed invalid casts (e.g. from varchar, to double, etc.) and produced incorrect results when casting to varchar. Casting 1 second interval used to return '1000' instead of '0 00:00:01.000'. Fixes facebookincubator#9384 Differential Revision: D55796600
fd78e99
to
16c13b3
Compare
This pull request was exported from Phabricator. Differential Revision: D55796600 |
Summary: CAST expression used to handle INTERVAL DAY TO SECOND values as BIGINT. This allowed invalid casts (e.g. from varchar, to double, etc.) and produced incorrect results when casting to varchar. Casting 1 second interval used to return '1000' instead of '0 00:00:01.000'. Fixes facebookincubator#9384 Differential Revision: D55796600
16c13b3
to
7ec7832
Compare
This pull request was exported from Phabricator. Differential Revision: D55796600 |
@amitkdutta Amit, thank you for review.
Fixed. Would you take another look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbasmanova LGTM. Thanks!
velox/expression/ConstantExpr.cpp
Outdated
[[fallthrough]]; | ||
case TypeKind::BIGINT: { | ||
if (vector.type()->isIntervalDayTime()) { | ||
auto intervalVector = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: auto*
velox/expression/CastExpr.cpp
Outdated
INTERVAL_DAY_TIME()->valueToString(inputFlatVector->valueAt(row)); | ||
auto writer = exec::StringWriter<>(resultFlatVector, row); | ||
writer.resize(output.size()); | ||
std::memcpy(writer.data(), output.data(), output.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: ::memcpy
Summary: CAST expression used to handle INTERVAL DAY TO SECOND values as BIGINT. This allowed invalid casts (e.g. from varchar, to double, etc.) and produced incorrect results when casting to varchar. Casting 1 second interval used to return '1000' instead of '0 00:00:01.000'. Fixes facebookincubator#9384 Reviewed By: xiaoxmeng Differential Revision: D55796600
7ec7832
to
b7334a9
Compare
This pull request was exported from Phabricator. Differential Revision: D55796600 |
Summary: CAST expression used to handle INTERVAL DAY TO SECOND values as BIGINT. This allowed invalid casts (e.g. from varchar, to double, etc.) and produced incorrect results when casting to varchar. Casting 1 second interval used to return '1000' instead of '0 00:00:01.000'. Fixes facebookincubator#9384 Reviewed By: xiaoxmeng Differential Revision: D55796600
b7334a9
to
ed8a134
Compare
This pull request was exported from Phabricator. Differential Revision: D55796600 |
This pull request has been merged in 41bed84. |
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Summary: Pull Request resolved: facebookincubator#9386 CAST expression used to handle INTERVAL DAY TO SECOND values as BIGINT. This allowed invalid casts (e.g. from varchar, to double, etc.) and produced incorrect results when casting to varchar. Casting 1 second interval used to return '1000' instead of '0 00:00:01.000'. Fixes facebookincubator#9384 Reviewed By: xiaoxmeng Differential Revision: D55796600 fbshipit-source-id: 3d51a8eb18b434315bf9285f58b8f2cdbedca63d
Summary:
CAST expression used to handle INTERVAL DAY TO SECOND values as BIGINT. This allowed invalid casts (e.g. from varchar, to double, etc.) and produced incorrect results when casting to varchar.
Casting 1 second interval used to return '1000' instead of '0 00:00:01.000'.
Fixes #9384
Differential Revision: D55796600