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

Merging #23843 #25394

Merged
merged 12 commits into from
Jun 17, 2021
Merged

Merging #23843 #25394

merged 12 commits into from
Jun 17, 2021

Conversation

kitaisreal
Copy link
Collaborator

@kitaisreal kitaisreal commented Jun 17, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Original pull request #23843.

Changelog category (leave one):

  • New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Added function bitPositionsToArray. Closes #23792. Author [Kevin Wan] (@MaxWk).

@robot-clickhouse robot-clickhouse added doc-alert pr-feature Pull request with new product feature labels Jun 17, 2021
@kitaisreal kitaisreal changed the title Function bitPositionToArray merge Function bitPositionsToArray merge Jun 17, 2021
@Avogar Avogar self-assigned this Jun 17, 2021
auto & vec_from = col_from->getData();
size_t size = vec_from.size();
result_array_offsets_data.resize(size);
result_array_values_data.reserve(size * 2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why size * 2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like heuristic from original author, make sense to reserve additional size for positions before push back. There is no optimal solution probably. It will be most efficient if number has 2 bits set to 1.

|| (result_column = executeType<Int8>(in_column))
|| (result_column = executeType<Int16>(in_column))
|| (result_column = executeType<Int32>(in_column))
|| (result_column = executeType<Int64>(in_column))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

128, 256?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.


DataTypePtr getReturnTypeImpl(const DataTypes & arguments) const override
{
if (!isNativeInteger(arguments[0]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not native may be also ok?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also extend bitmask functions for 128, 256 bits.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@kitaisreal kitaisreal changed the title Function bitPositionsToArray merge Merging #23843 Jun 17, 2021
@kitaisreal kitaisreal merged commit b34b66c into master Jun 17, 2021
@kitaisreal kitaisreal deleted the function-bit-position-to-array-merge branch June 17, 2021 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-feature Pull request with new product feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Function to get position of non zero bits.
4 participants