You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ks...@apache.org on 2019/03/19 08:04:48 UTC

[arrow] branch master updated: ARROW-4910: [Rust] [DataFusion] Remove all uses of unimplemented!

This is an automated email from the ASF dual-hosted git repository.

kszucs pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 9292ac9  ARROW-4910: [Rust] [DataFusion] Remove all uses of unimplemented!
9292ac9 is described below

commit 9292ac9c7a3c547298541ed8c0e0cd962722ff2c
Author: Andy Grove <an...@gmail.com>
AuthorDate: Tue Mar 19 09:04:38 2019 +0100

    ARROW-4910: [Rust] [DataFusion] Remove all uses of unimplemented!
    
    This PR removes all uses of `unimplemented!` in DataFusion.
    
    It also refactors to replace two copies of `expr_to_field` and `exprlist_to_fields` with one copy in a new file `optimizer::utils` and also removes some dead code.
    
    There are no functional changes in this PR.
    
    Note that I had already made these changes under the larger PR https://github.com/apache/arrow/pull/3939 but I felt it would be better to break this up into smaller PRs.
    
    Author: Andy Grove <an...@gmail.com>
    Author: Andy Grove <an...@rms.com>
    
    Closes #3961 from andygrove/ARROW-4910 and squashes the following commits:
    
    d2e934d0 <Andy Grove> formatting
    0039b335 <Andy Grove> Address PR feedback
    51da9f0b <Andy Grove> Remove all uses of unimplemented
---
 rust/datafusion/src/execution/context.rs |  60 +++-------
 rust/datafusion/src/logicalplan.rs       | 166 ++++------------------------
 rust/datafusion/src/optimizer/mod.rs     |   1 +
 rust/datafusion/src/optimizer/utils.rs   | 184 +++++++++++++++++++++++++++++++
 rust/datafusion/src/sql/planner.rs       |  66 ++---------
 5 files changed, 232 insertions(+), 245 deletions(-)

diff --git a/rust/datafusion/src/execution/context.rs b/rust/datafusion/src/execution/context.rs
index 4413ff9..22c0263 100644
--- a/rust/datafusion/src/execution/context.rs
+++ b/rust/datafusion/src/execution/context.rs
@@ -37,6 +37,7 @@ use crate::execution::relation::{DataSourceRelation, Relation};
 use crate::logicalplan::*;
 use crate::optimizer::optimizer::OptimizerRule;
 use crate::optimizer::projection_push_down::ProjectionPushDown;
+use crate::optimizer::utils;
 use crate::sql::parser::{DFASTNode, DFParser};
 use crate::sql::planner::{SchemaProvider, SqlToRel};
 
@@ -79,7 +80,10 @@ impl ExecutionContext {
 
                 Ok(self.optimize(&plan)?)
             }
-            _ => unimplemented!(),
+            other => Err(ExecutionError::General(format!(
+                "Cannot create logical plan from {:?}",
+                other
+            ))),
         }
     }
 
@@ -161,7 +165,7 @@ impl ExecutionContext {
                 let input_schema = input_rel.as_ref().borrow().schema().clone();
 
                 let project_columns: Vec<Field> =
-                    exprlist_to_fields(&expr, &input_schema);
+                    utils::exprlist_to_fields(&expr, &input_schema)?;
 
                 let project_schema = Arc::new(Schema::new(project_columns));
 
@@ -198,10 +202,12 @@ impl ExecutionContext {
 
                 let mut output_fields: Vec<Field> = vec![];
                 for expr in group_expr {
-                    output_fields.push(expr_to_field(expr, input_schema.as_ref()));
+                    output_fields
+                        .push(utils::expr_to_field(expr, input_schema.as_ref())?);
                 }
                 for expr in aggr_expr {
-                    output_fields.push(expr_to_field(expr, input_schema.as_ref()));
+                    output_fields
+                        .push(utils::expr_to_field(expr, input_schema.as_ref())?);
                 }
                 let rel = AggregateRelation::new(
                     Arc::new(Schema::new(output_fields)),
@@ -246,51 +252,13 @@ impl ExecutionContext {
                 }
             }
 
-            _ => unimplemented!(),
+            _ => Err(ExecutionError::NotImplemented(
+                "Unsupported logical plan for execution".to_string(),
+            )),
         }
     }
 }
 
-/// Create field meta-data from an expression, for use in a result set schema
-pub fn expr_to_field(e: &Expr, input_schema: &Schema) -> Field {
-    match e {
-        Expr::Column(i) => input_schema.fields()[*i].clone(),
-        Expr::Literal(ref lit) => Field::new("lit", lit.get_datatype(), true),
-        Expr::ScalarFunction {
-            ref name,
-            ref return_type,
-            ..
-        } => Field::new(&name, return_type.clone(), true),
-        Expr::AggregateFunction {
-            ref name,
-            ref return_type,
-            ..
-        } => Field::new(&name, return_type.clone(), true),
-        Expr::Cast { ref data_type, .. } => Field::new("cast", data_type.clone(), true),
-        Expr::BinaryExpr {
-            ref left,
-            ref right,
-            ..
-        } => {
-            let left_type = left.get_type(input_schema);
-            let right_type = right.get_type(input_schema);
-            Field::new(
-                "binary_expr",
-                get_supertype(&left_type, &right_type).unwrap(),
-                true,
-            )
-        }
-        _ => unimplemented!("Cannot determine schema type for expression {:?}", e),
-    }
-}
-
-/// Create field meta-data from an expression, for use in a result set schema
-pub fn exprlist_to_fields(expr: &Vec<Expr>, input_schema: &Schema) -> Vec<Field> {
-    expr.iter()
-        .map(|e| expr_to_field(e, input_schema))
-        .collect()
-}
-
 struct ExecutionContextSchemaProvider {
     datasources: Rc<RefCell<HashMap<String, Rc<Table>>>>,
 }
@@ -303,6 +271,6 @@ impl SchemaProvider for ExecutionContextSchemaProvider {
     }
 
     fn get_function_meta(&self, _name: &str) -> Option<Arc<FunctionMeta>> {
-        unimplemented!()
+        None
     }
 }
diff --git a/rust/datafusion/src/logicalplan.rs b/rust/datafusion/src/logicalplan.rs
index 94ebb28..432165f 100644
--- a/rust/datafusion/src/logicalplan.rs
+++ b/rust/datafusion/src/logicalplan.rs
@@ -18,10 +18,12 @@
 //! Logical query plan
 
 use std::fmt;
-use std::fmt::{Error, Formatter};
 use std::sync::Arc;
 
-use arrow::datatypes::*;
+use arrow::datatypes::{DataType, Field, Schema};
+
+use crate::error::{ExecutionError, Result};
+use crate::optimizer::utils;
 
 /// Enumeration of supported function types (Scalar and Aggregate)
 #[derive(Serialize, Deserialize, Debug, Clone)]
@@ -91,14 +93,6 @@ pub enum Operator {
     NotLike,
 }
 
-impl Operator {
-    /// Get the result type of applying this operation to its left and right inputs
-    pub fn get_datatype(&self, l: &Expr, _r: &Expr, schema: &Schema) -> DataType {
-        //TODO: implement correctly, just go with left side for now
-        l.get_type(schema).clone()
-    }
-}
-
 /// ScalarValue enumeration
 #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
 pub enum ScalarValue {
@@ -133,8 +127,7 @@ impl ScalarValue {
             ScalarValue::Float32(_) => DataType::Float32,
             ScalarValue::Float64(_) => DataType::Float64,
             ScalarValue::Utf8(_) => DataType::Utf8,
-            ScalarValue::Struct(_) => unimplemented!(),
-            ScalarValue::Null => unimplemented!(),
+            _ => panic!("Cannot treat {:?} as scalar value", self),
         }
     }
 }
@@ -191,28 +184,22 @@ impl Expr {
                 ref left,
                 ref right,
                 ref op,
-            } => {
-                match op {
-                    Operator::Eq | Operator::NotEq => DataType::Boolean,
-                    Operator::Lt | Operator::LtEq => DataType::Boolean,
-                    Operator::Gt | Operator::GtEq => DataType::Boolean,
-                    Operator::And | Operator::Or => DataType::Boolean,
-                    _ => {
-                        let left_type = left.get_type(schema);
-                        let right_type = right.get_type(schema);
-                        get_supertype(&left_type, &right_type).unwrap_or(DataType::Utf8) //TODO ???
-                    }
+            } => match op {
+                Operator::Eq | Operator::NotEq => DataType::Boolean,
+                Operator::Lt | Operator::LtEq => DataType::Boolean,
+                Operator::Gt | Operator::GtEq => DataType::Boolean,
+                Operator::And | Operator::Or => DataType::Boolean,
+                _ => {
+                    let left_type = left.get_type(schema);
+                    let right_type = right.get_type(schema);
+                    utils::get_supertype(&left_type, &right_type).unwrap()
                 }
-            }
+            },
             Expr::Sort { ref expr, .. } => expr.get_type(schema),
         }
     }
 
-    pub fn cast_to(
-        &self,
-        cast_to_type: &DataType,
-        schema: &Schema,
-    ) -> Result<Expr, String> {
+    pub fn cast_to(&self, cast_to_type: &DataType, schema: &Schema) -> Result<Expr> {
         let this_type = self.get_type(schema);
         if this_type == *cast_to_type {
             Ok(self.clone())
@@ -222,10 +209,10 @@ impl Expr {
                 data_type: cast_to_type.clone(),
             })
         } else {
-            Err(format!(
+            Err(ExecutionError::General(format!(
                 "Cannot automatically convert {:?} to {:?}",
                 this_type, cast_to_type
-            ))
+            )))
         }
     }
 
@@ -279,7 +266,7 @@ impl Expr {
 }
 
 impl fmt::Debug for Expr {
-    fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             Expr::Column(i) => write!(f, "#{}", i),
             Expr::Literal(v) => write!(f, "{:?}", v),
@@ -382,7 +369,7 @@ impl LogicalPlan {
 }
 
 impl LogicalPlan {
-    fn fmt_with_indent(&self, f: &mut Formatter, indent: usize) -> Result<(), Error> {
+    fn fmt_with_indent(&self, f: &mut fmt::Formatter, indent: usize) -> fmt::Result {
         if indent > 0 {
             writeln!(f)?;
             for _ in 0..indent {
@@ -458,122 +445,11 @@ impl LogicalPlan {
 }
 
 impl fmt::Debug for LogicalPlan {
-    fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         self.fmt_with_indent(f, 0)
     }
 }
 
-//TODO move to Arrow DataType impl?
-pub fn get_supertype(l: &DataType, r: &DataType) -> Option<DataType> {
-    match _get_supertype(l, r) {
-        Some(dt) => Some(dt),
-        None => match _get_supertype(r, l) {
-            Some(dt) => Some(dt),
-            None => None,
-        },
-    }
-}
-
-fn _get_supertype(l: &DataType, r: &DataType) -> Option<DataType> {
-    use self::DataType::*;
-    match (l, r) {
-        (UInt8, Int8) => Some(Int8),
-        (UInt8, Int16) => Some(Int16),
-        (UInt8, Int32) => Some(Int32),
-        (UInt8, Int64) => Some(Int64),
-
-        (UInt16, Int16) => Some(Int16),
-        (UInt16, Int32) => Some(Int32),
-        (UInt16, Int64) => Some(Int64),
-
-        (UInt32, Int32) => Some(Int32),
-        (UInt32, Int64) => Some(Int64),
-
-        (UInt64, Int64) => Some(Int64),
-
-        (Int8, UInt8) => Some(Int8),
-
-        (Int16, UInt8) => Some(Int16),
-        (Int16, UInt16) => Some(Int16),
-
-        (Int32, UInt8) => Some(Int32),
-        (Int32, UInt16) => Some(Int32),
-        (Int32, UInt32) => Some(Int32),
-
-        (Int64, UInt8) => Some(Int64),
-        (Int64, UInt16) => Some(Int64),
-        (Int64, UInt32) => Some(Int64),
-        (Int64, UInt64) => Some(Int64),
-
-        (UInt8, UInt8) => Some(UInt8),
-        (UInt8, UInt16) => Some(UInt16),
-        (UInt8, UInt32) => Some(UInt32),
-        (UInt8, UInt64) => Some(UInt64),
-        (UInt8, Float32) => Some(Float32),
-        (UInt8, Float64) => Some(Float64),
-
-        (UInt16, UInt8) => Some(UInt16),
-        (UInt16, UInt16) => Some(UInt16),
-        (UInt16, UInt32) => Some(UInt32),
-        (UInt16, UInt64) => Some(UInt64),
-        (UInt16, Float32) => Some(Float32),
-        (UInt16, Float64) => Some(Float64),
-
-        (UInt32, UInt8) => Some(UInt32),
-        (UInt32, UInt16) => Some(UInt32),
-        (UInt32, UInt32) => Some(UInt32),
-        (UInt32, UInt64) => Some(UInt64),
-        (UInt32, Float32) => Some(Float32),
-        (UInt32, Float64) => Some(Float64),
-
-        (UInt64, UInt8) => Some(UInt64),
-        (UInt64, UInt16) => Some(UInt64),
-        (UInt64, UInt32) => Some(UInt64),
-        (UInt64, UInt64) => Some(UInt64),
-        (UInt64, Float32) => Some(Float32),
-        (UInt64, Float64) => Some(Float64),
-
-        (Int8, Int8) => Some(Int8),
-        (Int8, Int16) => Some(Int16),
-        (Int8, Int32) => Some(Int32),
-        (Int8, Int64) => Some(Int64),
-        (Int8, Float32) => Some(Float32),
-        (Int8, Float64) => Some(Float64),
-
-        (Int16, Int8) => Some(Int16),
-        (Int16, Int16) => Some(Int16),
-        (Int16, Int32) => Some(Int32),
-        (Int16, Int64) => Some(Int64),
-        (Int16, Float32) => Some(Float32),
-        (Int16, Float64) => Some(Float64),
-
-        (Int32, Int8) => Some(Int32),
-        (Int32, Int16) => Some(Int32),
-        (Int32, Int32) => Some(Int32),
-        (Int32, Int64) => Some(Int64),
-        (Int32, Float32) => Some(Float32),
-        (Int32, Float64) => Some(Float64),
-
-        (Int64, Int8) => Some(Int64),
-        (Int64, Int16) => Some(Int64),
-        (Int64, Int32) => Some(Int64),
-        (Int64, Int64) => Some(Int64),
-        (Int64, Float32) => Some(Float32),
-        (Int64, Float64) => Some(Float64),
-
-        (Float32, Float32) => Some(Float32),
-        (Float32, Float64) => Some(Float64),
-        (Float64, Float32) => Some(Float64),
-        (Float64, Float64) => Some(Float64),
-
-        (Utf8, Utf8) => Some(Utf8),
-
-        (Boolean, Boolean) => Some(Boolean),
-
-        _ => None,
-    }
-}
-
 pub fn can_coerce_from(left: &DataType, other: &DataType) -> bool {
     use self::DataType::*;
     match left {
diff --git a/rust/datafusion/src/optimizer/mod.rs b/rust/datafusion/src/optimizer/mod.rs
index 27b853f..f507f5a 100644
--- a/rust/datafusion/src/optimizer/mod.rs
+++ b/rust/datafusion/src/optimizer/mod.rs
@@ -19,3 +19,4 @@
 
 pub mod optimizer;
 pub mod projection_push_down;
+pub mod utils;
diff --git a/rust/datafusion/src/optimizer/utils.rs b/rust/datafusion/src/optimizer/utils.rs
new file mode 100644
index 0000000..be97859
--- /dev/null
+++ b/rust/datafusion/src/optimizer/utils.rs
@@ -0,0 +1,184 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Collection of utility functions that are leveraged by the query optimizer rules
+
+use arrow::datatypes::{DataType, Field, Schema};
+
+use crate::error::{ExecutionError, Result};
+use crate::logicalplan::Expr;
+
+/// Create field meta-data from an expression, for use in a result set schema
+pub fn expr_to_field(e: &Expr, input_schema: &Schema) -> Result<Field> {
+    match e {
+        Expr::Column(i) => Ok(input_schema.fields()[*i].clone()),
+        Expr::Literal(ref lit) => Ok(Field::new("lit", lit.get_datatype(), true)),
+        Expr::ScalarFunction {
+            ref name,
+            ref return_type,
+            ..
+        } => Ok(Field::new(&name, return_type.clone(), true)),
+        Expr::AggregateFunction {
+            ref name,
+            ref return_type,
+            ..
+        } => Ok(Field::new(&name, return_type.clone(), true)),
+        Expr::Cast { ref data_type, .. } => {
+            Ok(Field::new("cast", data_type.clone(), true))
+        }
+        Expr::BinaryExpr {
+            ref left,
+            ref right,
+            ..
+        } => {
+            let left_type = left.get_type(input_schema);
+            let right_type = right.get_type(input_schema);
+            Ok(Field::new(
+                "binary_expr",
+                get_supertype(&left_type, &right_type).unwrap(),
+                true,
+            ))
+        }
+        _ => Err(ExecutionError::NotImplemented(format!(
+            "Cannot determine schema type for expression {:?}",
+            e
+        ))),
+    }
+}
+
+/// Create field meta-data from an expression, for use in a result set schema
+pub fn exprlist_to_fields(expr: &Vec<Expr>, input_schema: &Schema) -> Result<Vec<Field>> {
+    expr.iter()
+        .map(|e| expr_to_field(e, input_schema))
+        .collect()
+}
+
+/// Given two datatypes, determine the supertype that both types can safely be cast to
+pub fn get_supertype(l: &DataType, r: &DataType) -> Result<DataType> {
+    match _get_supertype(l, r) {
+        Some(dt) => Ok(dt),
+        None => match _get_supertype(r, l) {
+            Some(dt) => Ok(dt),
+            None => Err(ExecutionError::InternalError(format!(
+                "Failed to determine supertype of {:?} and {:?}",
+                l, r
+            ))),
+        },
+    }
+}
+
+/// Given two datatypes, determine the supertype that both types can safely be cast to
+fn _get_supertype(l: &DataType, r: &DataType) -> Option<DataType> {
+    use arrow::datatypes::DataType::*;
+    match (l, r) {
+        (UInt8, Int8) => Some(Int8),
+        (UInt8, Int16) => Some(Int16),
+        (UInt8, Int32) => Some(Int32),
+        (UInt8, Int64) => Some(Int64),
+
+        (UInt16, Int16) => Some(Int16),
+        (UInt16, Int32) => Some(Int32),
+        (UInt16, Int64) => Some(Int64),
+
+        (UInt32, Int32) => Some(Int32),
+        (UInt32, Int64) => Some(Int64),
+
+        (UInt64, Int64) => Some(Int64),
+
+        (Int8, UInt8) => Some(Int8),
+
+        (Int16, UInt8) => Some(Int16),
+        (Int16, UInt16) => Some(Int16),
+
+        (Int32, UInt8) => Some(Int32),
+        (Int32, UInt16) => Some(Int32),
+        (Int32, UInt32) => Some(Int32),
+
+        (Int64, UInt8) => Some(Int64),
+        (Int64, UInt16) => Some(Int64),
+        (Int64, UInt32) => Some(Int64),
+        (Int64, UInt64) => Some(Int64),
+
+        (UInt8, UInt8) => Some(UInt8),
+        (UInt8, UInt16) => Some(UInt16),
+        (UInt8, UInt32) => Some(UInt32),
+        (UInt8, UInt64) => Some(UInt64),
+        (UInt8, Float32) => Some(Float32),
+        (UInt8, Float64) => Some(Float64),
+
+        (UInt16, UInt8) => Some(UInt16),
+        (UInt16, UInt16) => Some(UInt16),
+        (UInt16, UInt32) => Some(UInt32),
+        (UInt16, UInt64) => Some(UInt64),
+        (UInt16, Float32) => Some(Float32),
+        (UInt16, Float64) => Some(Float64),
+
+        (UInt32, UInt8) => Some(UInt32),
+        (UInt32, UInt16) => Some(UInt32),
+        (UInt32, UInt32) => Some(UInt32),
+        (UInt32, UInt64) => Some(UInt64),
+        (UInt32, Float32) => Some(Float32),
+        (UInt32, Float64) => Some(Float64),
+
+        (UInt64, UInt8) => Some(UInt64),
+        (UInt64, UInt16) => Some(UInt64),
+        (UInt64, UInt32) => Some(UInt64),
+        (UInt64, UInt64) => Some(UInt64),
+        (UInt64, Float32) => Some(Float32),
+        (UInt64, Float64) => Some(Float64),
+
+        (Int8, Int8) => Some(Int8),
+        (Int8, Int16) => Some(Int16),
+        (Int8, Int32) => Some(Int32),
+        (Int8, Int64) => Some(Int64),
+        (Int8, Float32) => Some(Float32),
+        (Int8, Float64) => Some(Float64),
+
+        (Int16, Int8) => Some(Int16),
+        (Int16, Int16) => Some(Int16),
+        (Int16, Int32) => Some(Int32),
+        (Int16, Int64) => Some(Int64),
+        (Int16, Float32) => Some(Float32),
+        (Int16, Float64) => Some(Float64),
+
+        (Int32, Int8) => Some(Int32),
+        (Int32, Int16) => Some(Int32),
+        (Int32, Int32) => Some(Int32),
+        (Int32, Int64) => Some(Int64),
+        (Int32, Float32) => Some(Float32),
+        (Int32, Float64) => Some(Float64),
+
+        (Int64, Int8) => Some(Int64),
+        (Int64, Int16) => Some(Int64),
+        (Int64, Int32) => Some(Int64),
+        (Int64, Int64) => Some(Int64),
+        (Int64, Float32) => Some(Float32),
+        (Int64, Float64) => Some(Float64),
+
+        (Float32, Float32) => Some(Float32),
+        (Float32, Float64) => Some(Float64),
+        (Float64, Float32) => Some(Float64),
+        (Float64, Float64) => Some(Float64),
+
+        (Utf8, _) => Some(Utf8),
+        (_, Utf8) => Some(Utf8),
+
+        (Boolean, Boolean) => Some(Boolean),
+
+        _ => None,
+    }
+}
diff --git a/rust/datafusion/src/sql/planner.rs b/rust/datafusion/src/sql/planner.rs
index 45e2f8e..8c84bb3 100644
--- a/rust/datafusion/src/sql/planner.rs
+++ b/rust/datafusion/src/sql/planner.rs
@@ -22,6 +22,7 @@ use std::sync::Arc;
 
 use crate::error::*;
 use crate::logicalplan::*;
+use crate::optimizer::utils;
 
 use arrow::datatypes::*;
 
@@ -110,8 +111,10 @@ impl SqlToRel {
                     let mut all_fields: Vec<Expr> = group_expr.clone();
                     aggr_expr.iter().for_each(|x| all_fields.push(x.clone()));
 
-                    let aggr_schema =
-                        Schema::new(exprlist_to_fields(&all_fields, input_schema));
+                    let aggr_schema = Schema::new(utils::exprlist_to_fields(
+                        &all_fields,
+                        input_schema,
+                    )?);
 
                     //TODO: selection, projection, everything else
                     Ok(Arc::new(LogicalPlan::Aggregate {
@@ -126,10 +129,9 @@ impl SqlToRel {
                         _ => input.clone(),
                     };
 
-                    let projection_schema = Arc::new(Schema::new(exprlist_to_fields(
-                        &expr,
-                        input_schema.as_ref(),
-                    )));
+                    let projection_schema = Arc::new(Schema::new(
+                        utils::exprlist_to_fields(&expr, input_schema.as_ref())?,
+                    ));
 
                     let projection = LogicalPlan::Projection {
                         expr: expr,
@@ -234,9 +236,7 @@ impl SqlToRel {
             }
 
             &ASTNode::SQLWildcard => {
-                //                schema.columns().iter().enumerate()
-                //                    .map(|(i,c)| Ok(Expr::Column(i))).collect()
-                unimplemented!("SQL wildcard operator is not supported in projection - please use explicit column names")
+                Err(ExecutionError::NotImplemented("SQL wildcard operator is not supported in projection - please use explicit column names".to_string()))
             }
 
             &ASTNode::SQLCast {
@@ -284,13 +284,13 @@ impl SqlToRel {
                 let left_type = left_expr.get_type(schema);
                 let right_type = right_expr.get_type(schema);
 
-                match get_supertype(&left_type, &right_type) {
-                    Some(supertype) => Ok(Expr::BinaryExpr {
+                match utils::get_supertype(&left_type, &right_type) {
+                    Ok(supertype) => Ok(Expr::BinaryExpr {
                         left: Arc::new(left_expr.cast_to(&supertype, schema)?),
                         op: operator,
                         right: Arc::new(right_expr.cast_to(&supertype, schema)?),
                     }),
-                    None => {
+                    Err(_) => {
                         return Err(ExecutionError::General(format!(
                             "No common supertype found for binary operator {:?} \
                              with input types {:?} and {:?}",
@@ -397,48 +397,6 @@ pub fn convert_data_type(sql: &SQLType) -> Result<DataType> {
     }
 }
 
-/// Derive field meta-data for an expression, for use in creating schemas that result from
-/// evaluating expressions against an input schema.
-pub fn expr_to_field(e: &Expr, input_schema: &Schema) -> Field {
-    match e {
-        Expr::Column(i) => input_schema.fields()[*i].clone(),
-        Expr::Literal(ref lit) => Field::new("lit", lit.get_datatype(), true),
-        Expr::ScalarFunction {
-            ref name,
-            ref return_type,
-            ..
-        } => Field::new(name, return_type.clone(), true),
-        Expr::AggregateFunction {
-            ref name,
-            ref return_type,
-            ..
-        } => Field::new(name, return_type.clone(), true),
-        Expr::Cast { ref data_type, .. } => Field::new("cast", data_type.clone(), true),
-        Expr::BinaryExpr {
-            ref left,
-            ref right,
-            ..
-        } => {
-            let left_type = left.get_type(input_schema);
-            let right_type = right.get_type(input_schema);
-            Field::new(
-                "binary_expr",
-                get_supertype(&left_type, &right_type).unwrap(),
-                true,
-            )
-        }
-        _ => unimplemented!("Cannot determine schema type for expression {:?}", e),
-    }
-}
-
-/// Derive field meta-data for a list of expressions, for use in creating schemas that result from
-/// evaluating expressions against an input schema.
-pub fn exprlist_to_fields(expr: &Vec<Expr>, input_schema: &Schema) -> Vec<Field> {
-    expr.iter()
-        .map(|e| expr_to_field(e, input_schema))
-        .collect()
-}
-
 #[cfg(test)]
 mod tests {