Implement BIT_SHIFT_LEFT operation #1286

codernineteen · 2023-07-14T07:49:14Z

From the issue #1271,
Added below key features to several modules in core directory.

a way to translate the result of sqlparser
a execute path for shift_left operation
a definition of the operator '<<' as 'ShiftLeft'
new variant in LiteralError
test cases for above implementations

Body : Defined a new type to support bit shift operation and added a logic to pass from sqlparesr to gluesql ast builder

Body : Basically the structure of function is similar to other binary operations. But i added new vriant of LiteralError to make error-handling more sense

Body : This needs to make the statement executable.

coveralls · 2023-07-14T14:19:11Z

Pull Request Test Coverage Report for Build 5629113870

294 of 296 (99.32%) changed or added relevant lines in 8 files are covered.
17 unchanged lines in 5 files lost coverage.
Overall coverage decreased (-0.07%) to 98.638%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
test-suite/src/bitwise_shift.rs	40	42	95.24%

Files with Coverage Reduction	New Missed Lines	%
core/src/ast/function.rs	1	99.89%
core/src/executor/evaluate/function.rs	1	97.72%
core/src/plan/expr/function.rs	1	99.58%
core/src/executor/evaluate/error.rs	7	0%
core/src/translate/mod.rs	7	96.39%

Totals
Change from base Build 5550958766:	-0.07%
Covered Lines:	43951
Relevant Lines:	44558

💛 - Coveralls

test-suite/src/lib.rs

…bit-operation To apply requirements in pull request review

…ion" This reverts commit e5c9707. The purpose of this commit is to discard unrelated changes in pull request

Body: Added explicit error handling, also handled exceptional cases

core/src/ast/operator.rs

core/src/data/value/mod.rs

core/src/executor/evaluate/evaluated.rs

codernineteen

https://docs.rs/num-traits/latest/num_traits/ops/checked/trait.CheckedShl.html

According to above docs, it is true that 'checked_shl' was implemented for most of primitive integer types. However it seems that 'rhs' parameter for checked_shl is 'u32' type.
When i ran the test cases i wrote in suite, i always got the I64 Value type as a result. That's why i added this conversion funtionality into this module.
If there is anything i misunderstood , pls feedback )

test-suite/src/lib.rs

panarch · 2023-07-17T15:18:42Z

https://docs.rs/num-traits/latest/num_traits/ops/checked/trait.CheckedShl.html

We don't need to use num-traits because the primitive types already support checked_shl by default.
ref. https://doc.rust-lang.org/std/primitive.i32.html#method.checked_shl

it would be better to see the doc of rust primitive integer types.
each of them returns the same type of itself.

1. shift_left -> bitwise_shift_left 2. ShiftLeft -> BitwiseShiftLeft

I separated rhs match expression from original match pattern so that it can remove duplicated parts in the previous implementation

…efore

i also covered some missed lines

core/src/data/value/mod.rs

Also reflected the changes into related files

1. Removed 'validate_bitwise_shift_rhs' because it was only being used by this function 2. Deleted duplicated error handling because we can treat it in ok_or_else parenthesis when the 'checked_shl' fail

core/src/data/literal.rs

core/src/ast_builder/expr/binary_op.rs

core/src/data/value/mod.rs

core/src/data/literal.rs

Following review request, i added if statement to keep consistency if rhs is Null value by return Ok(Null)

1. Separated test cases into additional unit test case block. 2. Added new error variant into 'LiteralError' enum 3. Edit the implementation detail of bitwise_shift_left to handle explicit error handling

New error variant list : - BitwiseNonIntegerOperand for a operand that is Number literal, but not an integer type - BitwiseNonNumberLiteral for non number literal type - BitwiseOperationOverflow for overflow exception case - ImpossibleConversion when it is impossible to cast a value to another type

panarch

Looks all great! Thanks a lot for the contribution 👍 👍

codernineteen added 5 commits July 14, 2023 16:25

Added bit shift left operation

5bbb842

Body : Defined a new type to support bit shift operation and added a logic to pass from sqlparesr to gluesql ast builder

Implemented shift left function for literal type

03a5fc7

Body : Basically the structure of function is similar to other binary operations. But i added new vriant of LiteralError to make error-handling more sense

Added shift_left function in Evaluated impl block

49aec9e

Body : This needs to make the statement executable.

Added ShiftLeft variant into NumericBinaryOperator

d3a256f

Test cases to inspect the functionality of 'ShiftLeft' operation

e5c9707

ever0de linked an issue Jul 14, 2023 that may be closed by this pull request

Implement BIT_SHIFT function #1271

Closed

Added missed definition of 'shift_left' function

13dfcfd

panarch requested changes Jul 14, 2023

View reviewed changes

test-suite/src/lib.rs Outdated Show resolved Hide resolved

test-suite/src/lib.rs Outdated Show resolved Hide resolved

panarch requested review from ever0de and devgony July 14, 2023 14:22

panarch assigned codernineteen Jul 14, 2023

panarch added the enhancement New feature or request label Jul 14, 2023

codernineteen added 6 commits July 14, 2023 23:53

Added missed definition of 'shift_left' function

c564418

Merge remote-tracking branch 'origin/issue-bit-operation' into issue-…

c9637af

…bit-operation To apply requirements in pull request review

Revert "Test cases to inspect the functionality of 'ShiftLeft' operat…

ff73c45

…ion" This reverts commit e5c9707. The purpose of this commit is to discard unrelated changes in pull request

Test cases for bit shift left operation and its usage.

d04c7da

Edit typo in the variant word(Uncompatible -> Incompatible)

72492d4

Implement shift_left method for value type

b72de59

Body: Added explicit error handling, also handled exceptional cases

codernineteen requested a review from panarch July 17, 2023 02:17

panarch requested changes Jul 17, 2023

View reviewed changes

core/src/ast/operator.rs Outdated Show resolved Hide resolved

core/src/data/value/mod.rs Outdated Show resolved Hide resolved

core/src/executor/evaluate/evaluated.rs Outdated Show resolved Hide resolved

codernineteen commented Jul 17, 2023

View reviewed changes

test-suite/src/lib.rs Outdated Show resolved Hide resolved

codernineteen added 6 commits July 19, 2023 14:39

Renamed shift_left function with 'bitwise' prefix

d0c1204

1. shift_left -> bitwise_shift_left 2. ShiftLeft -> BitwiseShiftLeft

Refactored bitwise_shift_left implementation

2d01ad8

I separated rhs match expression from original match pattern so that it can remove duplicated parts in the previous implementation

Modified test cases corresponding to renaming and rectoring commits b…

0fe10ea

…efore

Fixed an error in Literal test case

12e2f2f

Added missing conversion from Bigdecimal to i64

d896d59

i also covered some missed lines

Covered missed lines in bitwise_shift_usage and evaluate error handling

fbbcd49

panarch reviewed Jul 20, 2023

View reviewed changes

core/src/data/value/mod.rs Outdated Show resolved Hide resolved

codernineteen added 3 commits July 20, 2023 14:40

Renamed bit_shift.rs to bitwise_shift.rs.

52597c9

Also reflected the changes into related files

Changed type of rhs field to U32

b904744

Refactored bitwise_shift_left implementation according to review.

3cdd114

1. Removed 'validate_bitwise_shift_rhs' because it was only being used by this function 2. Deleted duplicated error handling because we can treat it in ok_or_else parenthesis when the 'checked_shl' fail

panarch requested changes Jul 20, 2023

View reviewed changes

panarch reviewed Jul 20, 2023

View reviewed changes

core/src/data/literal.rs Outdated Show resolved Hide resolved

codernineteen added 4 commits July 20, 2023 20:54

Added bitwise_ prefix to function name

62e2935

Added if statement to check if rhs is Null

50c366e

Following review request, i added if statement to keep consistency if rhs is Null value by return Ok(Null)

Refactored literal code related to bitwise_shift_left

c865865

1. Separated test cases into additional unit test case block. 2. Added new error variant into 'LiteralError' enum 3. Edit the implementation detail of bitwise_shift_left to handle explicit error handling

panarch self-requested a review July 22, 2023 08:02

panarch approved these changes Jul 22, 2023

View reviewed changes

panarch merged commit 7cd1c5d into gluesql:main Jul 22, 2023
9 checks passed

This was referenced Aug 30, 2023

Implement bit shift right operator #1391

Closed

Bitwise Shift Right Operator Implementation #1394

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement BIT_SHIFT_LEFT operation #1286

Implement BIT_SHIFT_LEFT operation #1286

codernineteen commented Jul 14, 2023

coveralls commented Jul 14, 2023 •

edited

codernineteen left a comment

panarch commented Jul 17, 2023

panarch left a comment

Implement BIT_SHIFT_LEFT operation #1286

Implement BIT_SHIFT_LEFT operation #1286

Conversation

codernineteen commented Jul 14, 2023

coveralls commented Jul 14, 2023 • edited

Pull Request Test Coverage Report for Build 5629113870

💛 - Coveralls

codernineteen left a comment

Choose a reason for hiding this comment

panarch commented Jul 17, 2023

panarch left a comment

Choose a reason for hiding this comment

coveralls commented Jul 14, 2023 •

edited