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.

Sign up for GitHub

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

Jump to bottom

fix: Literal comparison with BinaryOperator #1397

Merged
merged 23 commits into from
Sep 11, 2023
Merged

fix: Literal comparison with BinaryOperator #1397

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
525f15a
feat: Implement TAKE function
ding-co Jul 13, 2023
f29afd2
test: Write integration test for TAKE function
ding-co Jul 13, 2023
d7be1a0
update: Take core/ast format whitespace
ding-co Jul 13, 2023
471e74b
update: Take test case using single use for import
ding-co Jul 13, 2023
4e69cbf
Merge remote-tracking branch 'upstream/main'
ding-co Jul 14, 2023
4be7cf5
refactor: Take fn match expression, variable shadowing
ding-co Jul 14, 2023
32ab2c0
feat: Add TAKE plan expr test code
ding-co Jul 14, 2023
7221fb6
update: Take fn name param added for error variable
ding-co Jul 14, 2023
9a617e4
update: Take fn, test case for handling null value
ding-co Jul 14, 2023
5df4e81
Merge remote-tracking branch 'upstream/main'
ding-co Jul 18, 2023
cf6c624
Merge remote-tracking branch 'upstream/main'
ding-co Jul 29, 2023
b588c9b
feat: Add AST Builder for Take function
ding-co Jul 29, 2023
3df1101
Merge remote-tracking branch 'upstream/main'
ding-co Jul 29, 2023
0a43fdc
Merge remote-tracking branch 'upstream/main'
ding-co Aug 5, 2023
e6a8aef
feat: Add Select without table for ast_builder
ding-co Aug 5, 2023
f167d51
update: Rename Select without table name fn
ding-co Aug 5, 2023
befa4ba
update: Rename test case fn && select without table test case
ding-co Aug 5, 2023
3c084dd
Merge remote-tracking branch 'upstream/main'
ding-co Sep 2, 2023
74ee4d3
fix: Add None conditional logic for literal comparison with binaryOpe…
ding-co Sep 2, 2023
8754345
feat: Add Literal comparison with BinaryOpeartor test cases
ding-co Sep 2, 2023
56aced0
refactor: Cargo clippy for is_none fn
ding-co Sep 2, 2023
7b788ee
Merge remote-tracking branch 'upstream/main'
ding-co Sep 10, 2023
2774069
refactor: BinaryOperator LtEq, GtEq using matches! macro
ding-co Sep 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions core/src/executor/evaluate/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,19 @@ pub fn binary_op<'a>(
BinaryOperator::Eq => cmp!(l.evaluate_eq(&r)),
BinaryOperator::NotEq => cmp!(!l.evaluate_eq(&r)),
BinaryOperator::Lt => cmp!(l.evaluate_cmp(&r) == Some(Ordering::Less)),
BinaryOperator::LtEq => cmp!(l.evaluate_cmp(&r) != Some(Ordering::Greater)),
BinaryOperator::LtEq => {
if l.evaluate_cmp(&r).is_none() {
return Ok(Evaluated::from(Value::Bool(false)));
}
cmp!(l.evaluate_cmp(&r) != Some(Ordering::Greater))
}
Copy link
Member

Choose a reason for hiding this comment

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

matches! would be useful. same approach can be applied for BinaryOperator::GtEq

BinaryOperator::LtEq => cmp!(matches!(
    l.evaluate_cmp(&r),
    Some(Ordering::Less) | Some(Ordering::Equal)
)),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wow! matches! macro thank you~

BinaryOperator::Gt => cmp!(l.evaluate_cmp(&r) == Some(Ordering::Greater)),
BinaryOperator::GtEq => cmp!(l.evaluate_cmp(&r) != Some(Ordering::Less)),
BinaryOperator::GtEq => {
if l.evaluate_cmp(&r).is_none() {
return Ok(Evaluated::from(Value::Bool(false)));
}
cmp!(l.evaluate_cmp(&r) != Some(Ordering::Less))
}
BinaryOperator::And => cond!(l && r),
BinaryOperator::Or => cond!(l || r),
BinaryOperator::Xor => cond!(l ^ r),
Expand Down
10 changes: 9 additions & 1 deletion test-suite/src/ordering.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::*;
use {crate::*, gluesql_core::prelude::Value::Bool};

test_case!(ordering, {
let g = get_tester!();
Expand Down Expand Up @@ -68,4 +68,12 @@ test_case!(ordering, {
for (num, sql) in test_cases {
g.count(sql, num).await;
}

// Literal comparison with BinaryOperator
g.test("select 1 < 'a' as test", Ok(select!(test Bool; false)))
.await;
g.test("select 1 >= 'a' as test", Ok(select!(test Bool; false)))
.await;
g.test("select 1 = 'a' as test", Ok(select!(test Bool; false)))
.await;
});