From 2ee282a18d178ec0124cc81f7c1a05f914f8dba9 Mon Sep 17 00:00:00 2001
From: Dan Spencer <dan@atomicpotato.net>
Date: Mon, 20 Apr 2015 02:38:21 -0600
Subject: [PATCH] Make proper distinction between ambiguous and nonexistent
 column name

Fixes #20
---
 src/queryplan/mod.rs    | 15 +++++++----
 src/queryplan/source.rs | 55 +++++++++++++++++++++++++++--------------
 2 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/src/queryplan/mod.rs b/src/queryplan/mod.rs
index d9a0883..60e61c5 100644
--- a/src/queryplan/mod.rs
+++ b/src/queryplan/mod.rs
@@ -15,7 +15,7 @@ use self::source::*;
 
 pub enum QueryPlanCompileError {
     TableDoesNotExist(Identifier),
-    /// ambiguous column name; two or more tables have a column of the same name
+    ColumnDoesNotExist(Identifier),
     AmbiguousColumnName(Identifier),
     BadIdentifier(String),
     BadStringLiteral(String),
@@ -34,6 +34,9 @@ impl fmt::Display for QueryPlanCompileError {
             &TableDoesNotExist(ref name) => {
                 write!(f, "table does not exist: {}", name)
             },
+            &ColumnDoesNotExist(ref name) => {
+                write!(f, "column does not exist: {}", name)
+            },
             &AmbiguousColumnName(ref name) => {
                 write!(f, "ambiguous column name: {}", name)
             },
@@ -510,8 +513,9 @@ where DB: 'a, <DB as DatabaseInfo>::Table: 'a
                 let column_identifier = try!(new_identifier(&s));
 
                 let (source_id, column_offset) = match scope.get_column_offset(&column_identifier) {
-                    Some(v) => v,
-                    None => return Err(QueryPlanCompileError::AmbiguousColumnName(column_identifier))
+                    GetColumnOffsetResult::One(v) => v,
+                    GetColumnOffsetResult::None => return Err(QueryPlanCompileError::ColumnDoesNotExist(column_identifier)),
+                    GetColumnOffsetResult::Ambiguous(..) => return Err(QueryPlanCompileError::AmbiguousColumnName(column_identifier))
                 };
 
                 groups_info.add_query_id(self.get_query_id_from_source_id(source_id));
@@ -526,8 +530,9 @@ where DB: 'a, <DB as DatabaseInfo>::Table: 'a
                 let column_identifier = try!(new_identifier(&s2));
 
                 let (source_id, column_offset) = match scope.get_table_column_offset(&table_identifier, &column_identifier) {
-                    Some(v) => v,
-                    None => return Err(QueryPlanCompileError::AmbiguousColumnName(column_identifier))
+                    GetColumnOffsetResult::One(v) => v,
+                    GetColumnOffsetResult::None => return Err(QueryPlanCompileError::ColumnDoesNotExist(column_identifier)),
+                    GetColumnOffsetResult::Ambiguous(..) => return Err(QueryPlanCompileError::AmbiguousColumnName(column_identifier))
                 };
 
                 groups_info.add_query_id(self.get_query_id_from_source_id(source_id));
diff --git a/src/queryplan/source.rs b/src/queryplan/source.rs
index adbe8e4..4cea07b 100644
--- a/src/queryplan/source.rs
+++ b/src/queryplan/source.rs
@@ -13,6 +13,12 @@ pub struct SourceScope<'a>
     table_aliases: Vec<Identifier>
 }
 
+pub enum GetColumnOffsetResult {
+    One((u32, u32)),
+    None,
+    Ambiguous(Vec<(u32, u32)>)
+}
+
 fn get_candidates<'a, I>(tables: I, name: &Identifier) -> Vec<(u32, u32)>
 where I: Iterator<Item=&'a TableOrSubquery>
 {
@@ -39,21 +45,38 @@ impl<'a> SourceScope<'a> {
 
     pub fn tables(&self) -> &[TableOrSubquery] { &self.tables }
 
-    pub fn get_column_offset(&self, column_name: &Identifier) -> Option<(u32, u32)> {
-        let mut candidates = get_candidates(self.tables.iter(), column_name);
 
-        candidates.extend(self.parent.and_then(|parent| {
-            parent.get_column_offset(column_name)
-        }).into_iter());
+    pub fn get_column_offset(&self, column_name: &Identifier) -> GetColumnOffsetResult {
+        let candidates = self.get_column_offsets(column_name);
 
-        if candidates.len() == 1 {
-            Some(candidates[0])
-        } else {
-            None
+        match candidates.len() {
+            0 => GetColumnOffsetResult::None,
+            1 => GetColumnOffsetResult::One(candidates[0]),
+            _ => GetColumnOffsetResult::Ambiguous(candidates)
         }
     }
 
-    pub fn get_table_column_offset(&self, table_name: &Identifier, column_name: &Identifier) -> Option<(u32, u32)> {
+    pub fn get_table_column_offset(&self, table_name: &Identifier, column_name: &Identifier) -> GetColumnOffsetResult {
+        let candidates = self.get_table_column_offsets(table_name, column_name);
+
+        match candidates.len() {
+            0 => GetColumnOffsetResult::None,
+            1 => GetColumnOffsetResult::One(candidates[0]),
+            _ => GetColumnOffsetResult::Ambiguous(candidates)
+        }
+    }
+
+    fn get_column_offsets(&self, column_name: &Identifier) -> Vec<(u32, u32)> {
+        let mut candidates = get_candidates(self.tables.iter(), column_name);
+
+        if let Some(parent) = self.parent {
+            candidates.extend(parent.get_column_offsets(column_name));
+        }
+
+        candidates
+    }
+
+    fn get_table_column_offsets(&self, table_name: &Identifier, column_name: &Identifier) -> Vec<(u32, u32)> {
         let tables = self.table_aliases.iter().enumerate().filter_map(|(i, name)| {
             if name == table_name { Some(&self.tables[i]) }
             else { None }
@@ -61,14 +84,10 @@ impl<'a> SourceScope<'a> {
 
         let mut candidates = get_candidates(tables, column_name);
 
-        candidates.extend(self.parent.and_then(|parent| {
-            parent.get_table_column_offset(table_name, column_name)
-        }).into_iter());
-
-        if candidates.len() == 1 {
-            Some(candidates[0])
-        } else {
-            None
+        if let Some(parent) = self.parent {
+            candidates.extend(parent.get_table_column_offsets(table_name, column_name));
         }
+
+        candidates
     }
 }