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

Add support for bitwise shift functions #722

Closed
martint opened this issue May 6, 2019 · 5 comments
Closed

Add support for bitwise shift functions #722

martint opened this issue May 6, 2019 · 5 comments
Labels

Comments

@martint
Copy link
Member

martint commented May 6, 2019

Presto currently supports bitwise functions and/or/xor/not. We should add support for bitwise left and right shift.

We need to consider how the function may or may not be sensitive to the length of the data type. From the original issue (prestodb/presto#4028):

Function that has tricky overflow issue:

BITWISE_LSH(7, 2, 4) = 12 -- treat 7 as a 4-bit signed number and left shift by 2,
                          -- truncate overflow

Function that are inherently sensitive to width of numbers:

BITWISE_LRSH(-8, 2, 5) = 6 -- treat -8 as a 5-bit signed number and lsh by 2
@martint
Copy link
Member Author

martint commented May 12, 2019

Additionally, we need to think about the following scenarios:

  • How does it behave for different types: TINYINT, SMALLINT, INTEGER, BIGINT?
  • How does it behave with respect to implicit casts?

Naming-wise, we may want to call them bitwise_left_shift and bitwise_right_shift

@martint
Copy link
Member Author

martint commented Jul 11, 2020

I think what we want are the following functions for each t in [TINYINT, SMALLINT, INTEGER, BIGINT]

bitwise_left_shift(value::t, bits::integer) :: t
bitwise_right_shift(value::t, bits::integer) :: t
bitwise_right_shift_signed(value::t, bits::integer) :: t

That would make the functions consistent with the implementations of shift operators for various types in traditional programming languages and databases that support shift operators. For each of the types in the list above, the values are interpreted as 2s complement signed 8-bit, 16-bit, 32-bit and 64-bit numbers, respectively. The _signed variant of right shift preserves the sign of the number (similar to >> in Java).

@prestosql/maintainers, @Lewuathe, thoughts?

@Lewuathe
Copy link
Member

@martint
I think having multiple dedicated functions for each type is a good idea. That would make the interface clearer.

But _shift_arithmetic seems to be straightforward instead of _shift_signed so that we can make it explicit that the function is for the arithmetic right shift.

bitwise_left_shift(value::t, bits::integer) :: t
bitwise_right_shift(value::t, bits::integer) :: t
bitwise_right_shift_arithmetic(value::t, bits::integer) :: t

@martint
Copy link
Member Author

martint commented Jul 13, 2020

The issue I have with arithmetic is that it's not immediately obvious to me that it means "extend sign". But it may be clear to others.

@martint
Copy link
Member Author

martint commented Jul 13, 2020

In any case, let's go with this plan. We can finalize the name later.

@martint martint mentioned this issue Aug 6, 2020
8 tasks
@ebyhr ebyhr closed this as completed Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants