Add Floor, Ceil, Round Function #291

tmdgusya · 2021-08-16T11:07:31Z

I'm done to add the functions below.

Ceil Func : Nearest integer greater than or equal to argument

example ) 11.2 -> 12
example ) -11.3 -> 11

Floor Func : Nearest integer less than or equal to argument

example ) 11.3 -> 11
example ) -11.3 -> -12
Rounds Func : v to s decimal places. Ties are broken by rounding away from zero.

example ) 11.6 -> 12
exmaple ) 11.2 -> 11

The ceil query receives one numeric parameter and executes ceil().

An error that will be ejected when a value other than the value that can be converted to float is received.

If the given value is convertible to float, then execute ceil(), if not convertible, return FunctionRequiresFloatValue error.

If the given value is convertible to float, then run round(), and if not convert, return the FunctionRequiresFloatValue error.

The round() function takes one argument that can be converted to a plot and runs round(), and returns an error if there are more than one or more non-convertible.

if the given value is convertible to float, then excute floor(), and if not convertible, return the FunctionRequiresFloatValue error.

Modify to pass cargo clipy

panarch

Awesome, clean and minimal 👍 👍 👍
Let's change little bit and merge it into the main!

panarch · 2021-08-17T06:17:30Z

src/tests/function/floor.rs

+        (
+            "SELECT FLOOR(0.3) AS floor FROM SingleItem",
+            Ok(select!(
+                "floor";
+                F64;
+                0.3_f64.floor()
+            )),
+        ),
+        (
+            "SELECT FLOOR(-0.8) AS floor FROM SingleItem",
+            Ok(select!(
+                "floor";
+                F64;
+                (-0.8_f64).floor()
+            )),
+        ),
+        (
+            "SELECT FLOOR(10) AS floor FROM SingleItem",
+            Ok(select!(
+                "floor";
+                F64;
+                f64::from(10).floor()
+            )),
+        ),


These test cases are identical from the view of our project.
It would be good to consider merging into a one. SELECT FLOOR(0.3), FLOOR(-0.8), FLOOR(10) FROM SingleItem

ref #289 (comment)

panarch · 2021-08-17T06:20:31Z

src/executor/evaluate/mod.rs

+        Function::Ceil(expr) => {
+            let number = match eval(expr).await?.try_into()? {
+                Value::F64(number) => Some(number),
+                Value::Str(s) => match f64::from_str(&s) {
+                    Ok(f) => Some(f),
+                    Err(_) => None,
+                },
+                Value::I64(number) => f64::from_i64(number),
+                _ => None,
+            };
+
+            match number {
+                Some(number) => Ok(Evaluated::from(Value::F64(number.ceil()))),
+                None => Err(EvaluateError::FunctionRequiresFloatValue("CEIL".to_owned()).into()),
+            }
+        }
+        Function::Round(expr) => {
+            let number = match eval(expr).await?.try_into()? {
+                Value::F64(number) => Some(number),
+                Value::Str(s) => match f64::from_str(&s) {
+                    Ok(f) => Some(f),
+                    Err(_) => None,
+                },
+                Value::I64(number) => f64::from_i64(number),
+                _ => None,
+            };
+
+            match number {
+                Some(number) => Ok(Evaluated::from(Value::F64(number.round()))),
+                None => Err(EvaluateError::FunctionRequiresFloatValue("ROUND".to_owned()).into()),
+            }
+        }


Good start! I think you also recognize there are some redundant codes which could be wrapped by a function.
Let's reduce redundancy in Ceil, Round and Floor.
Not just less redundancy helps programmers to write less codes, but also it is better because there's also less stuffs to be tested.

panarch · 2021-08-17T06:22:40Z

src/executor/evaluate/mod.rs

+                Value::Str(s) => match f64::from_str(&s) {
+                    Ok(f) => Some(f),
+                    Err(_) => None,
+                },


Let's handle Str input as an error FunctionRequiresFloatValue.
We can accept, I64 to F64 because it is also natively supported by Rust lang itself, but Str is not.
Users who need to run FLOOR using Str value, then they can use CAST function.

It is also better not to convert Err into non Err types.
ref #289 (comment)

rust native not support str to f64

use eval_to_float

ever0de · 2021-08-17T16:06:54Z

src/translate/function.rs

+        "CEIL" => {
+            check_len(name, args.len(), 1)?;
+
+            translate_expr(args[0])
+                .map(Function::Ceil)
+                .map(Box::new)
+                .map(Expr::Function)
+        }
+        "ROUND" => {
+            check_len(name, args.len(), 1)?;
+
+            translate_expr(args[0])
+                .map(Function::Round)
+                .map(Box::new)
+                .map(Expr::Function)
+        }
+        "FLOOR" => {
+            check_len(name, args.len(), 1)?;
+
+            translate_expr(args[0])
+                .map(Function::Floor)
+                .map(Box::new)
+                .map(Expr::Function)
+        }


I made a macro called func_with_one_arg.
You can add it comfortably.
Rebase and use the master.
Below is an example of usage.

func_with_one_arg!(Function::Ceil)

Can be used when it's a function and only has one argument

Oh thank you, i'll use that macro that can remove redundant code

move round

panarch

Awesome, eval_to_float looks really, really good :)
Thanks a lot! 🥇

panarch · 2021-08-20T06:33:27Z

resolve #264

tmdgusya added 14 commits August 16, 2021 19:35

add CEIL function to enum

03c824f

add CEIL Query in translate

b147339

The ceil query receives one numeric parameter and executes ceil().

add FunctionRequiresFloatValue

bfde144

An error that will be ejected when a value other than the value that can be converted to float is received.

implement CEIL Function

dec913c

If the given value is convertible to float, then execute ceil(), if not convertible, return FunctionRequiresFloatValue error.

style: cargo formatting

61e2e48

add ceil func test

746b7bf

add Round func to function enum

004336b

implements Round func

87bac24

If the given value is convertible to float, then run round(), and if not convert, return the FunctionRequiresFloatValue error.

add ROUND translate query

6aa4f4e

The round() function takes one argument that can be converted to a plot and runs round(), and returns an error if there are more than one or more non-convertible.

add round func test

dc78cb6

add Floor func to Function enum

e59d597

implement Floor Query

7532fa3

if the given value is convertible to float, then excute floor(), and if not convertible, return the FunctionRequiresFloatValue error.

add Floor func to translate query layer

1121c93

add floor function test

1af6787

panarch self-requested a review August 16, 2021 11:18

panarch assigned tmdgusya Aug 16, 2021

panarch added the enhancement New feature or request label Aug 16, 2021

fixed syntax

7ced6dc

Modify to pass cargo clipy

panarch requested changes Aug 17, 2021

View reviewed changes

tmdgusya added 9 commits August 17, 2021 20:10

refactor: merge similar cases in ceil

60940b2

fix: throw FunctionRequiresFloatValue if math func given str

5a97b5e

rust native not support str to f64

refactor: merge similar cases in ceil

14f18ec

refactor: merge similar cases in round

630d874

fix: not used f64::from() func

cda3d50

refactor: Remove duplicate code

5f6a735

use eval_to_float

test: return Null if given args null

65aa8d3

refactor: delete return statement

9b55d07

Merge branch 'main' of https://github.com/gluesql/gluesql

631b4d1

ever0de reviewed Aug 17, 2021

View reviewed changes

Merge branch 'main' of https://github.com/gluesql/gluesql

9e31912

tmdgusya added 3 commits August 18, 2021 19:18

refactor: use map function

66968a0

style: remove trailling whitespace

dfbd6d7

style: cargo fmt

3e46193

move round

tmdgusya requested a review from panarch August 18, 2021 15:49

panarch approved these changes Aug 19, 2021

View reviewed changes

panarch merged commit 35636a5 into gluesql:main Aug 19, 2021

panarch mentioned this pull request Aug 20, 2021

Add mathematical functions: DIV(), MOD(). #295

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Floor, Ceil, Round Function #291

Add Floor, Ceil, Round Function #291

tmdgusya commented Aug 16, 2021

panarch left a comment

panarch Aug 17, 2021

tmdgusya Aug 17, 2021

panarch Aug 17, 2021

panarch Aug 17, 2021

ever0de Aug 17, 2021

ever0de Aug 17, 2021

tmdgusya Aug 18, 2021

panarch left a comment

panarch commented Aug 20, 2021

Add Floor, Ceil, Round Function #291

Add Floor, Ceil, Round Function #291

Conversation

tmdgusya commented Aug 16, 2021

panarch left a comment

Choose a reason for hiding this comment

panarch Aug 17, 2021

Choose a reason for hiding this comment

tmdgusya Aug 17, 2021

Choose a reason for hiding this comment

panarch Aug 17, 2021

Choose a reason for hiding this comment

panarch Aug 17, 2021

Choose a reason for hiding this comment

ever0de Aug 17, 2021

Choose a reason for hiding this comment

ever0de Aug 17, 2021

Choose a reason for hiding this comment

tmdgusya Aug 18, 2021

Choose a reason for hiding this comment

panarch left a comment

Choose a reason for hiding this comment

panarch commented Aug 20, 2021