-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
Window view functional test failed on platform s390x #45375
Conversation
src/Interpreters/Aggregator.h
Outdated
FieldType casted_key = static_cast<FieldType>(key); | ||
const auto * key_holder = reinterpret_cast<const char *>(&casted_key); | ||
auto * column = static_cast<ColumnVectorHelper *>(key_columns[0]); | ||
column->insertRawData<sizeof(FieldType)>(key_holder); |
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.
It's not a good solution - at least it should be under if constexpr
or #ifdef
for endianness, but I would consider option to add offset to key_holder
if sizeof(FieldType) < sizeof(Key)
also, I'm not sure if there can't be a case sizeof(Key) < sizeof(FieldType) - it will be not good...
Co-authored-by: Yakov Olkhovskiy <99031427+yakov-olkhovskiy@users.noreply.github.com>
@Mergifyio update |
@SuzyWangIBMer could you please update your branch with master? we had some issues there which were fixed I hope... |
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
...
in the case key casted from uint64 to uint32, small impact for little endian platform but key value becomes zero in big endian case.
Documentation entry for user-facing changes