refine code

databendlabs · May 13, 2022 · e723b4e · e723b4e
1 parent 1923a1b
commit e723b4e
Show file tree

Hide file tree

Showing 9 changed files with 211 additions and 39 deletions.
diff --git a/query/src/sql/exec/expression_builder.rs b/query/src/sql/exec/expression_builder.rs
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use common_datavalues::DataSchemaRef;
+use common_datavalues::DataTypeImpl;
 use common_datavalues::DataValue;
 use common_exception::ErrorCode;
 use common_exception::Result;
@@ -67,7 +68,9 @@ impl<'a> ExpressionBuilder<'a> {
             Scalar::BoundColumnRef(BoundColumnRef { column }) => {
                 self.build_column_ref(column.index)
             }
-            Scalar::ConstantExpr(ConstantExpr { value }) => self.build_literal(value),
+            Scalar::ConstantExpr(ConstantExpr { value, data_type }) => {
+                self.build_literal(value, data_type)
+            }
             Scalar::ComparisonExpr(ComparisonExpr { op, left, right }) => {
                 self.build_binary_operator(left, right, op.to_func_name())
             }
@@ -134,8 +137,16 @@ impl<'a> ExpressionBuilder<'a> {
         )))
     }
 
-    pub fn build_literal(&self, data_value: &DataValue) -> Result<Expression> {
-        Ok(Expression::create_literal(data_value.clone()))
+    pub fn build_literal(
+        &self,
+        data_value: &DataValue,
+        data_type: &DataTypeImpl,
+    ) -> Result<Expression> {
+        Ok(Expression::Literal {
+            value: data_value.clone(),
+            column_name: None,
+            data_type: data_type.clone(),
+        })
     }
 
     pub fn build_binary_operator(

diff --git a/query/src/sql/planner/binder/limit.rs b/query/src/sql/planner/binder/limit.rs
@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use common_ast::ast::Expr;
+use common_datavalues::DataType;
 use common_exception::ErrorCode;
 use common_exception::Result;
 
@@ -34,8 +35,11 @@ impl<'a> Binder {
 
         let limit_cnt = match limit {
             Some(Expr::Literal { span: _, lit: x }) => {
-                let data = type_checker.parse_literal(x, None)?.as_u64()?;
-                Some(data as usize)
+                let (value, data_type) = type_checker.resolve_literal(x, None)?;
+                if !data_type.data_type_id().is_integer() {
+                    return Err(ErrorCode::IllegalDataType("Unsupported limit type"));
+                }
+                Some(value.as_u64()? as usize)
             }
             Some(_) => {
                 return Err(ErrorCode::IllegalDataType("Unsupported limit type"));
@@ -44,8 +48,11 @@ impl<'a> Binder {
         };
 
         let offset_cnt = if let Some(Expr::Literal { span: _, lit: x }) = offset {
-            let data = type_checker.parse_literal(x, None)?.as_u64()?;
-            data as usize
+            let (value, data_type) = type_checker.resolve_literal(x, None)?;
+            if !data_type.data_type_id().is_integer() {
+                return Err(ErrorCode::IllegalDataType("Unsupported limit type"));
+            }
+            value.as_u64()? as usize
         } else {
             0
         };

diff --git a/query/src/sql/planner/binder/select.rs b/query/src/sql/planner/binder/select.rs
@@ -224,11 +224,13 @@ impl<'a> Binder {
                 let expressions = args
                     .into_iter()
                     .map(|(scalar, _)| match scalar {
-                        Scalar::ConstantExpr(ConstantExpr { value }) => Ok(Expression::Literal {
-                            value: value.clone(),
-                            column_name: None,
-                            data_type: value.data_type(),
-                        }),
+                        Scalar::ConstantExpr(ConstantExpr { value, data_type }) => {
+                            Ok(Expression::Literal {
+                                value,
+                                column_name: None,
+                                data_type,
+                            })
+                        }
                         _ => Err(ErrorCode::UnImplement(format!(
                             "Unsupported table argument type: {:?}",
                             scalar

diff --git a/query/src/sql/planner/metadata.rs b/query/src/sql/planner/metadata.rs
@@ -124,7 +124,7 @@ impl Metadata {
 
     pub fn get_expr_display_string(&self, expr: &Expr) -> Result<String> {
         match expr {
-            Expr::ColumnRef { column, .. } => Ok(column.name.clone()),
+            Expr::ColumnRef { column, .. } => Ok(column.name.to_lowercase()),
 
             Expr::Literal { lit, .. } => Ok(format!("{lit}")),
 
@@ -157,17 +157,22 @@ impl Metadata {
 
             Expr::IsNull { expr, not, .. } => Ok(format!(
                 "{} IS {}NULL",
-                expr,
+                self.get_expr_display_string(expr)?,
                 if *not { "NOT " } else { "" }
             )),
 
             Expr::InList {
                 expr, list, not, ..
             } => {
                 let mut w = vec![];
-                write!(&mut w, "{} {}IN (", expr, if *not { "NOT " } else { "" })?;
+                write!(
+                    &mut w,
+                    "{} {}IN (",
+                    self.get_expr_display_string(expr)?,
+                    if *not { "NOT " } else { "" }
+                )?;
                 for (i, expr) in list.iter().enumerate() {
-                    write!(&mut w, "{}", expr)?;
+                    write!(&mut w, "{}", self.get_expr_display_string(expr)?)?;
                     if i < list.len() - 1 {
                         write!(&mut w, ", ")?;
                     }
@@ -185,21 +190,46 @@ impl Metadata {
                 ..
             } => Ok(format!(
                 "{} {}BETWEEN {} AND {}",
-                expr,
+                self.get_expr_display_string(expr)?,
                 if *not { "NOT " } else { "" },
-                low,
-                high
+                self.get_expr_display_string(low)?,
+                self.get_expr_display_string(high)?
             )),
 
             Expr::BinaryOp {
                 op, left, right, ..
-            } => Ok(format!("{} {} {}", left, op, right)),
+            } => Ok(format!(
+                "{} {} {}",
+                self.get_expr_display_string(left)?,
+                op,
+                self.get_expr_display_string(right)?
+            )),
 
-            Expr::UnaryOp { op, expr, .. } => Ok(format!("{} {}", op, expr)),
+            Expr::UnaryOp { op, expr, .. } => {
+                Ok(format!("{} {}", op, self.get_expr_display_string(expr)?))
+            }
 
             Expr::Cast {
                 expr, target_type, ..
-            } => Ok(format!("CAST({} AS {})", expr, target_type)),
+            } => Ok(format!(
+                "CAST({} AS {})",
+                self.get_expr_display_string(expr)?,
+                target_type
+            )),
+
+            Expr::TryCast {
+                expr, target_type, ..
+            } => Ok(format!(
+                "TRY_CAST({} AS {})",
+                self.get_expr_display_string(expr)?,
+                target_type
+            )),
+
+            Expr::Extract { field, expr, .. } => Ok(format!(
+                "EXTRACT({} FROM {})",
+                field,
+                self.get_expr_display_string(expr)?
+            )),
 
             Expr::Substring {
                 expr,
@@ -208,14 +238,14 @@ impl Metadata {
                 ..
             } => Ok(format!(
                 "SUBSTRING({}{}{})",
-                expr,
+                self.get_expr_display_string(expr)?,
                 if let Some(from) = substring_from {
-                    format!("FROM {}", from)
+                    format!("FROM {}", self.get_expr_display_string(from)?)
                 } else {
                     "".to_string()
                 },
                 if let Some(for_expr) = substring_for {
-                    format!("FOR {}", for_expr)
+                    format!("FOR {}", self.get_expr_display_string(for_expr)?)
                 } else {
                     "".to_string()
                 }

diff --git a/query/src/sql/planner/plans/scalar.rs b/query/src/sql/planner/plans/scalar.rs
@@ -72,11 +72,13 @@ impl ScalarExpr for BoundColumnRef {
 #[derive(Clone, PartialEq, Debug)]
 pub struct ConstantExpr {
     pub value: DataValue,
+
+    pub data_type: DataTypeImpl,
 }
 
 impl ScalarExpr for ConstantExpr {
     fn data_type(&self) -> DataTypeImpl {
-        self.value.data_type()
+        self.data_type.clone()
     }
 
     fn used_columns(&self) -> ColumnSet {