-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
SelectiveDecimalColumnReader::scanSpec_::valueHook_ may not be nullptr #11290
Comments
|
@Yuhta Thanks for your reply. I'd like to work on it. |
weiting-chen
pushed a commit
to weiting-chen/velox
that referenced
this issue
Nov 23, 2024
Summary: Currently, `int64_t` enables push-down for decimal type. This PR disables aggregate pushdown for decimal type regardless of c++ type. Fixes facebookincubator#11290 Pull Request resolved: facebookincubator#11298 Reviewed By: Yuhta Differential Revision: D65834210 Pulled By: kevinwilfong fbshipit-source-id: 422f7eda8f4184c6fa83055e7cf430ff5053d387
weiting-chen
added a commit
to oap-project/velox
that referenced
this issue
Nov 23, 2024
…ebookincubator#11298 (#505) * Fix NaN handling for min/max aggreates pushed down to scan (facebookincubator#10583) Summary: Pull Request resolved: facebookincubator#10583 This fixes min/max aggregates to handle NaN values correctly when they are pushed down to the scan operator. Specifically, the change ensures that NaN values are considered greater than infinity. Reviewed By: zacw7 Differential Revision: D60297934 fbshipit-source-id: 3398ba24d6fc70bbd0d8583d2949391b0824099c * Schema evolution support for reader value hooks (facebookincubator#10755) Summary: X-link: facebookincubator/nimble#72 Pull Request resolved: facebookincubator#10755 Currently reader value hook is not considering schema evolution at all, this change fix that. Reviewed By: kevinwilfong Differential Revision: D61229494 fbshipit-source-id: 729bb90611fb3164282b524376eda20985a30194 * Disable aggregate pushdown for decimal type (facebookincubator#11298) Summary: Currently, `int64_t` enables push-down for decimal type. This PR disables aggregate pushdown for decimal type regardless of c++ type. Fixes facebookincubator#11290 Pull Request resolved: facebookincubator#11298 Reviewed By: Yuhta Differential Revision: D65834210 Pulled By: kevinwilfong fbshipit-source-id: 422f7eda8f4184c6fa83055e7cf430ff5053d387 --------- Co-authored-by: Bikramjeet Vig <[email protected]> Co-authored-by: Jimmy Lu <[email protected]> Co-authored-by: NEUpanning <[email protected]>
athmaja-n
pushed a commit
to athmaja-n/velox
that referenced
this issue
Jan 10, 2025
Summary: Currently, `int64_t` enables push-down for decimal type. This PR disables aggregate pushdown for decimal type regardless of c++ type. Fixes facebookincubator#11290 Pull Request resolved: facebookincubator#11298 Reviewed By: Yuhta Differential Revision: D65834210 Pulled By: kevinwilfong fbshipit-source-id: 422f7eda8f4184c6fa83055e7cf430ff5053d387
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Bug description
Exception is caused by
SelectiveDecimalColumnReader::scanSpec_::valueHook_ != nullptr
when the physical type of decimal isint64_t
and aggregation push-down is enabled.I see that currently only int128_t doesn't enable push-down for
SelectiveDecimalColumnReader
, but decimal also can beint64_t
. Perhaps, we should letint64_t
do this as well.System information
Velox System Info v0.0.2
Commit: 33928e00
CMake Version: 3.29.6
System: Linux-3.10.0-862.mt20190308.130.el7.x86_64
Arch: x86_64
C++ Compiler: /opt/rh/devtoolset-10/root/usr/bin/c++
C++ Compiler Version: 10.2.1
C Compiler: /opt/rh/devtoolset-10/root/usr/bin/cc
C Compiler Version: 10.2.1
Relevant logs
No response
The text was updated successfully, but these errors were encountered: