Repository: gluesql/gluesql
Pull request #106 (2020-10-26 09:14:57)

Support UPPER & LOWER non-aggregating functions
It starts with `UPPER` and `LOWER` functions.

Let's add more non-aggregating functions!
It is now easy to implement by editing `evaluate_function` function in `executor/evaluate/mod.rs`.


Patch:
diff --git a/src/executor/aggregate/mod.rs b/src/executor/aggregate/mod.rs
index 00bdfbb0..58b3422f 100644
--- a/src/executor/aggregate/mod.rs
+++ b/src/executor/aggregate/mod.rs
@@ -21,12 +21,6 @@ pub use error::AggregateError;
 pub use hash::GroupKey;
 use state::State;
 
-#[derive(Iterator)]
-enum Aggregated<I1, I2> {
-    Applied(I1),
-    Skipped(I2),
-}
-
 pub struct Aggregate<'a, T: 'static + Debug> {
     storage: &'a dyn Store<T>,
     fields: &'a [SelectItem],
@@ -56,7 +50,13 @@ impl<'a, T: 'static + Debug> Aggregate<'a, T> {
         &self,
         rows: impl Iterator<Item = Result<Rc<BlendContext<'a>>>>,
     ) -> Result<impl Iterator<Item = Result<AggregateContext<'a>>>> {
-        if !self.check_aggregate() {
+        #[derive(Iterator)]
+        enum Aggregated<I1, I2> {
+            Applied(I1),
+            Skipped(I2),
+        }
+
+        if !self.check_aggregate()? {
             let rows = rows.map(|row| {
                 row.map(|blend_context| AggregateContext {
                     aggregated: None,
@@ -123,16 +123,20 @@ impl<'a, T: 'static + Debug> Aggregate<'a, T> {
         Ok(Aggregated::Applied(rows))
     }
 
-    fn check_aggregate(&self) -> bool {
+    fn check_aggregate(&self) -> Result<bool> {
         if !self.group_by.is_empty() {
-            return true;
+            return Ok(true);
         }
 
-        self.fields.iter().any(|field| match field {
-            SelectItem::UnnamedExpr(expr) => check(expr),
-            SelectItem::ExprWithAlias { expr, .. } => check(expr),
-            _ => false,
-        })
+        self.fields
+            .iter()
+            .map(|field| match field {
+                SelectItem::UnnamedExpr(expr) => check(expr),
+                SelectItem::ExprWithAlias { expr, .. } => check(expr),
+                _ => Ok(false),
+            })
+            .collect::<Result<Vec<bool>>>()
+            .map(|checked| checked.into_iter().any(|c| c))
     }
 }
 
@@ -202,15 +206,20 @@ fn aggregate<'a>(
     }
 }
 
-fn check(expr: &Expr) -> bool {
-    match expr {
+fn check(expr: &Expr) -> Result<bool> {
+    let checked = match expr {
         Expr::Between {
             expr, low, high, ..
-        } => check(expr) || check(low) || check(high),
-        Expr::BinaryOp { left, right, .. } => check(left) || check(right),
-        Expr::UnaryOp { expr, .. } => check(expr),
-        Expr::Nested(expr) => check(expr),
-        Expr::Function(_) => true,
+        } => check(expr)? || check(low)? || check(high)?,
+        Expr::BinaryOp { left, right, .. } => check(left)? || check(right)?,
+        Expr::UnaryOp { expr, .. } => check(expr)?,
+        Expr::Nested(expr) => check(expr)?,
+        Expr::Function(func) => matches!(
+            get_name(&func.name)?.to_uppercase().as_str(),
+            "COUNT" | "SUM" | "MAX" | "MIN" | "AVG"
+        ),
         _ => false,
-    }
+    };
+
+    Ok(checked)
 }
diff --git a/src/executor/blend.rs b/src/executor/blend.rs
index b7b6fc00..4191880d 100644
--- a/src/executor/blend.rs
+++ b/src/executor/blend.rs
@@ -1,7 +1,7 @@
 use im_rc::HashMap;
 use iter_enum::Iterator;
 use serde::Serialize;
-use std::convert::TryFrom;
+use std::convert::TryInto;
 use std::fmt::Debug;
 use std::iter::once;
 use std::rc::Rc;
@@ -10,7 +10,7 @@ use thiserror::Error;
 use sqlparser::ast::{Function, SelectItem};
 
 use super::context::{AggregateContext, BlendContext};
-use super::evaluate::{evaluate, Evaluated};
+use super::evaluate::evaluate;
 use crate::data::{get_name, Row, Value};
 use crate::result::Result;
 use crate::store::Store;
@@ -57,6 +57,17 @@ impl<'a, T: 'static + Debug> Blend<'a, T> {
             };
         }
 
+        macro_rules! try_into {
+            ($v: expr) => {
+                match $v {
+                    Ok(v) => v,
+                    Err(e) => {
+                        return err!(e);
+                    }
+                }
+            };
+        }
+
         let filter_context = context.concat_into(None);
 
         self.fields
@@ -68,12 +79,7 @@ impl<'a, T: 'static + Debug> Blend<'a, T> {
                     Blended::All(values)
                 }
                 SelectItem::QualifiedWildcard(alias) => {
-                    let table_alias = match get_name(alias) {
-                        Ok(alias) => alias,
-                        Err(e) => {
-                            return err!(e);
-                        }
-                    };
+                    let table_alias = try_into!(get_name(alias));
 
                     match context.get_alias_values(table_alias) {
                         Some(values) => Blended::AllInTable(values.into_iter().map(Ok)),
@@ -83,21 +89,9 @@ impl<'a, T: 'static + Debug> Blend<'a, T> {
                 SelectItem::UnnamedExpr(expr) | SelectItem::ExprWithAlias { expr, .. } => {
                     let filter_context = filter_context.as_ref().map(Rc::clone);
 
-                    let value =
-                        match evaluate(self.storage, filter_context, aggregated.as_ref(), expr) {
-                            Ok(value) => value,
-                            Err(e) => {
-                                return err!(e);
-                            }
-                        };
-
-                    let value = match value {
-                        Evaluated::LiteralRef(v) => Value::try_from(v),
-                        Evaluated::Literal(v) => Value::try_from(&v),
-                        Evaluated::StringRef(v) => Ok(Value::Str(v.to_string())),
-                        Evaluated::ValueRef(v) => Ok(v.clone()),
-                        Evaluated::Value(v) => Ok(v),
-                    };
+                    let value = evaluate(self.storage, filter_context, aggregated.as_ref(), expr)
+                        .map(|evaluated| evaluated.try_into());
+                    let value = try_into!(value);
 
                     Blended::Single(once(value))
                 }
diff --git a/src/executor/evaluate/error.rs b/src/executor/evaluate/error.rs
index 9f6c9f64..b4a271c4 100644
--- a/src/executor/evaluate/error.rs
+++ b/src/executor/evaluate/error.rs
@@ -10,6 +10,17 @@ pub enum EvaluateError {
     #[error("literal add on non-numeric")]
     LiteralAddOnNonNumeric,
 
+    #[error("function is not supported: {0}")]
+    FunctionNotSupported(String),
+
+    #[error("function requires string value: {0}")]
+    FunctionRequiresStringValue(String),
+
+    #[error(
+        "number of function parameters not matching (expected: {expected:?}, found: {found:?})"
+    )]
+    NumberOfFunctionParamsNotMatching { expected: usize, found: usize },
+
     #[error("unsupported compound identifier {0}")]
     UnsupportedCompoundIdentifier(String),
 
@@ -22,12 +33,6 @@ pub enum EvaluateError {
     #[error("unreachable literal arithmetic")]
     UnreachableLiteralArithmetic,
 
-    #[error("unreachable, aggregated field not found {0}")]
-    UnreachableAggregatedField(String),
-
-    #[error("unreachable, aggregated field does not exist")]
-    UnreachableEmptyAggregated,
-
     #[error("unimplemented")]
     Unimplemented,
 }
diff --git a/src/executor/evaluate/evaluated.rs b/src/executor/evaluate/evaluated.rs
index a7243ac1..5c46e75a 100644
--- a/src/executor/evaluate/evaluated.rs
+++ b/src/executor/evaluate/evaluated.rs
@@ -1,10 +1,11 @@
 use std::cmp::Ordering;
+use std::convert::{TryFrom, TryInto};
 
 use sqlparser::ast::Value as AstValue;
 
 use crate::data;
 use crate::data::Value;
-use crate::result::Result;
+use crate::result::{Error, Result};
 
 use super::EvaluateError;
 
@@ -144,6 +145,20 @@ fn literal_partial_cmp(a: &AstValue, b: &AstValue) -> Option<Ordering> {
     }
 }
 
+impl TryInto<Value> for Evaluated<'_> {
+    type Error = Error;
+
+    fn try_into(self) -> Result<Value> {
+        match self {
+            Evaluated::LiteralRef(v) => Value::try_from(v),
+            Evaluated::Literal(v) => Value::try_from(&v),
+            Evaluated::StringRef(v) => Ok(Value::Str(v.to_string())),
+            Evaluated::ValueRef(v) => Ok(v.clone()),
+            Evaluated::Value(v) => Ok(v),
+        }
+    }
+}
+
 impl<'a> Evaluated<'a> {
     pub fn add(&self, other: &Evaluated<'a>) -> Result<Evaluated<'a>> {
         use Evaluated::*;
diff --git a/src/executor/evaluate/mod.rs b/src/executor/evaluate/mod.rs
index 2aa4e225..613496ec 100644
--- a/src/executor/evaluate/mod.rs
+++ b/src/executor/evaluate/mod.rs
@@ -2,6 +2,7 @@ mod error;
 mod evaluated;
 
 use im_rc::HashMap;
+use std::convert::TryInto;
 use std::fmt::Debug;
 use std::rc::Rc;
 
@@ -9,7 +10,7 @@ use sqlparser::ast::{BinaryOperator, Expr, Function, Value as AstValue};
 
 use super::context::FilterContext;
 use super::select::select;
-use crate::data::Value;
+use crate::data::{get_name, Value};
 use crate::result::Result;
 use crate::store::Store;
 
@@ -92,11 +93,63 @@ pub fn evaluate<'a, T: 'static + Debug>(
         }
         Expr::Function(func) => aggregated
             .as_ref()
-            .map(|aggregated| match aggregated.get(func) {
-                Some(value) => Ok(Evaluated::Value(value.clone())),
-                None => Err(EvaluateError::UnreachableAggregatedField(func.to_string()).into()),
-            })
-            .unwrap_or_else(|| Err(EvaluateError::UnreachableEmptyAggregated.into())),
+            .map(|aggr| aggr.get(func))
+            .flatten()
+            .map_or_else(
+                || {
+                    let context = context.as_ref().map(Rc::clone);
+
+                    evaluate_function(storage, context, aggregated, func)
+                },
+                |value| Ok(Evaluated::Value(value.clone())),
+            ),
         _ => Err(EvaluateError::Unimplemented.into()),
     }
 }
+
+fn evaluate_function<'a, T: 'static + Debug>(
+    storage: &'a dyn Store<T>,
+    context: Option<Rc<FilterContext<'a>>>,
+    aggregated: Option<&HashMap<&Function, Value>>,
+    func: &'a Function,
+) -> Result<Evaluated<'a>> {
+    let eval = |expr| {
+        let context = context.as_ref().map(Rc::clone);
+
+        evaluate(storage, context, aggregated, expr)
+    };
+
+    let Function { name, args, .. } = func;
+
+    match get_name(name)?.to_uppercase().as_str() {
+        name @ "LOWER" | name @ "UPPER" => {
+            let convert = |s: String| {
+                if name == "LOWER" {
+                    s.to_lowercase()
+                } else {
+                    s.to_uppercase()
+                }
+            };
+
+            if args.len() != 1 {
+                return Err(EvaluateError::NumberOfFunctionParamsNotMatching {
+                    expected: 1,
+                    found: args.len(),
+                }
+                .into());
+            }
+
+            let expr = &args[0];
+            let value: Value = match eval(expr)?.try_into()? {
+                Value::Str(s) => Value::Str(convert(s)),
+                Value::OptStr(s) => Value::OptStr(s.map(convert)),
+                _ => {
+                    return Err(EvaluateError::FunctionRequiresStringValue(name.to_string()).into());
+                }
+            };
+
+            Ok(Evaluated::Value(value))
+        }
+        name => Err(EvaluateError::FunctionNotSupported(name.to_owned()).into()),
+    }
+}


Test patch:
diff --git a/src/tests/aggregate.rs b/src/tests/aggregate.rs
index 531b743e..74845818 100644
--- a/src/tests/aggregate.rs
+++ b/src/tests/aggregate.rs
@@ -74,8 +74,8 @@ pub fn aggregate(mut tester: impl tests::Tester) {
             "SELECT SUM(id.name.ok) FROM Item;",
         ),
         (
-            AggregateError::UnsupportedAggregation("WHATEVER".to_owned()).into(),
-            "SELECT WHATEVER(*) FROM Item;",
+            AggregateError::UnsupportedAggregation("AVG".to_owned()).into(),
+            "SELECT AVG(*) FROM Item;",
         ),
         (
             AggregateError::OnlyIdentifierAllowed.into(),
diff --git a/src/tests/function.rs b/src/tests/function.rs
new file mode 100644
index 00000000..a4732e8c
--- /dev/null
+++ b/src/tests/function.rs
@@ -0,0 +1,67 @@
+use crate::*;
+
+pub fn function(mut tester: impl tests::Tester) {
+    let mut run = |sql| tester.run(sql);
+
+    use Value::Str;
+
+    let test_cases = vec![
+        ("CREATE TABLE Item (name TEXT)", Ok(Payload::Create)),
+        (
+            r#"INSERT INTO Item VALUES ("abcd"), ("Abcd"), ("ABCD")"#,
+            Ok(Payload::Insert(3)),
+        ),
+        (
+            r#"SELECT name FROM Item WHERE LOWER(name) = "abcd";"#,
+            Ok(select!(
+                name Str;
+                "abcd".to_owned();
+                "Abcd".to_owned();
+                "ABCD".to_owned()
+            )),
+        ),
+        (
+            "SELECT LOWER(name), UPPER(name) FROM Item;",
+            Ok(select!(
+                "LOWER(name)"      | "UPPER(name)"
+                Str                | Str;
+                "abcd".to_owned()    "ABCD".to_owned();
+                "abcd".to_owned()    "ABCD".to_owned();
+                "abcd".to_owned()    "ABCD".to_owned()
+            )),
+        ),
+        (
+            r#"
+            SELECT
+                LOWER("Abcd") as lower,
+                UPPER("abCd") as upper
+            FROM Item LIMIT 1;
+            "#,
+            Ok(select!(
+                lower             | upper
+                Str               | Str;
+                "abcd".to_owned()   "ABCD".to_owned()
+            )),
+        ),
+        (
+            "SELECT LOWER() FROM Item",
+            Err(EvaluateError::NumberOfFunctionParamsNotMatching {
+                expected: 1,
+                found: 0,
+            }
+            .into()),
+        ),
+        (
+            "SELECT LOWER(1) FROM Item",
+            Err(EvaluateError::FunctionRequiresStringValue("LOWER".to_owned()).into()),
+        ),
+        (
+            "SELECT WHATEVER(1) FROM Item",
+            Err(EvaluateError::FunctionNotSupported("WHATEVER".to_owned()).into()),
+        ),
+    ];
+
+    test_cases
+        .into_iter()
+        .for_each(|(sql, expected)| assert_eq!(expected, run(sql)));
+}
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
index ebc1301a..7598a6f9 100644
--- a/src/tests/mod.rs
+++ b/src/tests/mod.rs
@@ -9,6 +9,7 @@ pub mod default;
 pub mod drop_table;
 pub mod error;
 pub mod filter;
+pub mod function;
 pub mod join;
 pub mod migrate;
 pub mod nested_select;
@@ -73,6 +74,7 @@ macro_rules! generate_tests {
         glue!(sql_types, sql_types::sql_types);
         glue!(synthesize, synthesize::synthesize);
         glue!(filter, filter::filter);
+        glue!(function, function::function);
 
         generate_alter_table_tests!();
     };