FIX: Fixes several issues with DATABASE functions

Fixes #547

base/src/functions/database.rs

@@ -1,6 +1,9 @@
+use chrono::Datelike;
+
 use crate::{
     calc_result::CalcResult,
     expressions::{parser::Node, token::Error, types::CellReferenceIndex},
+    formatter::dates::date_to_serial_number,
     Model,
 };
 
@@ -28,6 +31,15 @@ impl Model {
             Err(e) => return e,
         };
 
+        if db_right.row <= db_left.row {
+            // no data rows
+            return CalcResult::Error {
+                error: Error::VALUE,
+                origin: cell,
+                message: "No data rows in database".to_string(),
+            };
+        }
+
         let mut sum = 0.0f64;
         let mut count = 0usize;
 
@@ -82,6 +94,15 @@ impl Model {
             Err(e) => return e,
         };
 
+        if db_right.row <= db_left.row {
+            // no data rows
+            return CalcResult::Error {
+                error: Error::VALUE,
+                origin: cell,
+                message: "No data rows in database".to_string(),
+            };
+        }
+
         let mut count = 0usize;
         let mut row = db_left.row + 1; // skip header
         while row <= db_right.row {
@@ -123,10 +144,19 @@ impl Model {
             Err(e) => return e,
         };
 
+        if db_right.row <= db_left.row {
+            // no data rows
+            return CalcResult::Error {
+                error: Error::VALUE,
+                origin: cell,
+                message: "No data rows in database".to_string(),
+            };
+        }
+
         let mut result: Option<CalcResult> = None;
         let mut matches = 0usize;
 
-        let mut row = db_left.row + 1; // skip header
+        let mut row = db_left.row + 1;
         while row <= db_right.row {
             if self.db_row_matches_criteria(db_left, db_right, row, criteria) {
                 matches += 1;
@@ -187,7 +217,16 @@ impl Model {
             Err(e) => return e,
         };
 
-        let mut sum = 0.0f64;
+        if db_right.row <= db_left.row {
+            // no data rows
+            return CalcResult::Error {
+                error: Error::VALUE,
+                origin: cell,
+                message: "No data rows in database".to_string(),
+            };
+        }
+
+        let mut sum = 0.0;
 
         // skip header
         let mut row = db_left.row + 1;
@@ -220,42 +259,80 @@ impl Model {
         field_arg: &Node,
         cell: CellReferenceIndex,
     ) -> Result<i32, CalcResult> {
-        // If numeric -> index
+        let field_column_name = match self.evaluate_node_in_context(field_arg, cell) {
-        if let Ok(n) = self.get_number(field_arg, cell) {
+            CalcResult::String(s) => s.to_lowercase(),
-            let idx = if n < 1.0 {
+            CalcResult::Number(index) => {
-                n.ceil() as i32
+                let index = index.floor() as i32;
-            } else {
+                if index < 1 || db_left.column + index - 1 > db_right.column {
-                n.floor() as i32
-            };
-            if idx < 1 || db_left.column + idx - 1 > db_right.column {
                     return Err(CalcResult::Error {
                         error: Error::VALUE,
                         origin: cell,
                         message: "Field index out of range".to_string(),
                     });
                 }
-            return Ok(db_left.column + idx - 1);
+                return Ok(db_left.column + index - 1);
+            }
+            CalcResult::Boolean(b) => {
+                return if b {
+                    Ok(1)
+                } else {
+                    // Index 0 is out of range
+                    Err(CalcResult::Error {
+                        error: Error::VALUE,
+                        origin: cell,
+                        message: "Invalid field specifier".to_string(),
+                    })
+                };
+            }
+            error @ CalcResult::Error { .. } => {
+                return Err(error);
+            }
+            CalcResult::Range { .. } => {
+                return Err(CalcResult::Error {
+                    error: Error::NIMPL,
+                    origin: cell,
+                    message: "Arrays not supported yet".to_string(),
+                })
+            }
+            CalcResult::EmptyCell | CalcResult::EmptyArg => "".to_string(),
+            CalcResult::Array(_) => {
+                return Err(CalcResult::Error {
+                    error: Error::NIMPL,
+                    origin: cell,
+                    message: "Arrays not supported yet".to_string(),
+                })
             }
-
-        // Otherwise treat as header name
-        let wanted = match self.get_string(field_arg, cell) {
-            Ok(s) => s.to_lowercase(),
-            Err(e) => return Err(e),
         };
 
-        let mut col = db_left.column;
+        // We search in the database a column whose header matches field_column_name
-        while col <= db_right.column {
+        for column in db_left.column..=db_right.column {
             let v = self.evaluate_cell(CellReferenceIndex {
                 sheet: db_left.sheet,
                 row: db_left.row,
-                column: col,
+                column,
             });
-            if let CalcResult::String(s) = v {
+            match &v {
-                if s.to_lowercase() == wanted {
+                CalcResult::String(s) => {
-                    return Ok(col);
+                    if s.to_lowercase() == field_column_name {
+                        return Ok(column);
                     }
                 }
-            col += 1;
+                CalcResult::Number(n) => {
+                    if field_column_name == n.to_string() {
+                        return Ok(column);
+                    }
+                }
+                CalcResult::Boolean(b) => {
+                    if field_column_name == b.to_string() {
+                        return Ok(column);
+                    }
+                }
+                CalcResult::Error { .. }
+                | CalcResult::Range { .. }
+                | CalcResult::EmptyCell
+                | CalcResult::EmptyArg
+                | CalcResult::Array(_) => {}
+            }
         }
 
         Err(CalcResult::Error {
@@ -278,53 +355,84 @@ impl Model {
 
         // Read criteria headers (first row of criteria range)
         // Map header name (lowercased) -> db column (if exists)
-        let mut crit_cols: Vec<Option<i32>> = Vec::new();
+        let mut crit_cols: Vec<i32> = Vec::new();
-        let mut col = c_left.column;
+        let mut header_count = 0;
-        while col <= c_right.column {
+        // We cover the criteria table:
-            let h = self.evaluate_cell(CellReferenceIndex {
+        //  headerA | headerB | ...
+        //  critA1  | critA2  | ...
+        //  critB1  | critB2  | ...
+        //  ...
+        for column in c_left.column..=c_right.column {
+            let cell = CellReferenceIndex {
                 sheet: c_left.sheet,
                 row: c_left.row,
-                column: col,
+                column,
-            });
+            };
-            let db_col = if let CalcResult::String(s) = h {
+            let criteria_header = self.evaluate_cell(cell);
+            if let Ok(s) = self.cast_to_string(criteria_header, cell) {
+                // Non-empty string header. If the header is non string we skip it
+                header_count += 1;
                 let wanted = s.to_lowercase();
-                // Find corresponding DB column
+
-                let mut db_c = db_left.column;
+                // Find corresponding Database column
-                let mut found: Option<i32> = None;
+                let mut found = false;
-                while db_c <= db_right.column {
+                for db_column in db_left.column..=db_right.column {
-                    let hdr = self.evaluate_cell(CellReferenceIndex {
+                    let db_header = self.evaluate_cell(CellReferenceIndex {
                         sheet: db_left.sheet,
                         row: db_left.row,
-                        column: db_c,
+                        column: db_column,
                     });
-                    if let CalcResult::String(hs) = hdr {
+                    if let Ok(hs) = self.cast_to_string(db_header, cell) {
                         if hs.to_lowercase() == wanted {
-                            found = Some(db_c);
+                            crit_cols.push(db_column);
+                            found = true;
                             break;
                         }
                     }
-                    db_c += 1;
                 }
-                found
+                if !found {
-            } else {
+                    // that means the criteria column has no matching DB column
-                None
+                    // If the criteria condition is empty then we remove this condition
+                    // otherwise this condition can never be satisfied
+                    // We evaluate all criteria rows to see if any is non-empty
+                    let mut has_non_empty = false;
+                    for r in (c_left.row + 1)..=c_right.row {
+                        let ccell = self.evaluate_cell(CellReferenceIndex {
+                            sheet: c_left.sheet,
+                            row: r,
+                            column,
+                        });
+                        if !matches!(ccell, CalcResult::EmptyCell | CalcResult::EmptyArg) {
+                            has_non_empty = true;
+                            break;
+                        }
+                    }
+                    if has_non_empty {
+                        // This criteria column can never be satisfied
+                        header_count -= 1;
+                    }
+                }
             };
-            crit_cols.push(db_col);
-            col += 1;
         }
 
-        // If no criteria rows (only headers), everything matches
         if c_right.row <= c_left.row {
+            // If no criteria rows (only headers), everything matches
             return true;
         }
 
+        if header_count == 0 {
+            // If there are not "String" headers, nothing matches
+            // NB: There might be String headers that do not match any DB columns,
+            // in that case everything matches.
+            return false;
+        }
+
         // Evaluate each criteria row (OR)
-        let mut r = c_left.row + 1;
+        for r in (c_left.row + 1)..=c_right.row {
-        while r <= c_right.row {
             // AND across columns for this criteria row
             let mut and_ok = true;
 
-            for (offset, maybe_db_col) in crit_cols.iter().enumerate() {
+            for (offset, db_col) in crit_cols.iter().enumerate() {
                 // Criteria cell
                 let ccell = self.evaluate_cell(CellReferenceIndex {
                     sheet: c_left.sheet,
@@ -337,17 +445,11 @@ impl Model {
                     continue;
                 }
 
-                // Header without mapping -> ignore this criteria column (Excel ignores unknown headers)
-                let db_col = match maybe_db_col {
-                    Some(c) => *c,
-                    None => continue,
-                };
-
                 // Database value for this row/column
                 let db_val = self.evaluate_cell(CellReferenceIndex {
                     sheet: db_left.sheet,
                     row,
-                    column: db_col,
+                    column: *db_col,
                 });
 
                 if !self.criteria_cell_matches(&db_val, &ccell) {
@@ -360,8 +462,6 @@ impl Model {
                 // This criteria row satisfied (OR)
                 return true;
             }
-
-            r += 1;
         }
 
         // none matched
@@ -373,44 +473,67 @@ impl Model {
     fn criteria_cell_matches(&self, db_val: &CalcResult, crit_cell: &CalcResult) -> bool {
         // Convert the criteria cell to a string for operator parsing if possible,
         // otherwise fall back to equality via compare_values.
-        let crit_str_opt = match crit_cell {
+
-            CalcResult::String(s) => Some(s.clone()),
+        let mut criteria = match crit_cell {
-            CalcResult::Number(n) => Some(n.to_string()),
+            CalcResult::String(s) => s.trim().to_string(),
-            CalcResult::Boolean(b) => Some(if *b {
+            CalcResult::Number(n) => {
-                "TRUE".to_string()
+                // treat as equality with number
-            } else {
+                return match db_val {
-                "FALSE".to_string()
+                    CalcResult::Number(v) => (*v - *n).abs() <= f64::EPSILON,
-            }),
+                    _ => false,
-            CalcResult::EmptyCell | CalcResult::EmptyArg => return true,
+                };
+            }
+            CalcResult::Boolean(b) => {
+                // check equality with boolean
+                return match db_val {
+                    CalcResult::Boolean(v) => *v == *b,
+                    _ => false,
+                };
+            }
+            CalcResult::EmptyCell | CalcResult::EmptyArg => "".to_string(),
             CalcResult::Error { .. } => return false,
             CalcResult::Range { .. } | CalcResult::Array(_) => return false,
         };
 
-        if crit_str_opt.is_none() {
-            return compare_values(db_val, crit_cell) == 0;
-        }
-        let mut crit = match crit_str_opt {
-            Some(s) => s.trim().to_string(),
-            None => return false,
-        };
-
         // Detect operator prefix
         let mut op = "="; // default equality (with wildcard semantics for strings)
         let prefixes = ["<>", ">=", "<=", ">", "<", "="];
         for p in prefixes.iter() {
-            if crit.starts_with(p) {
+            if criteria.starts_with(p) {
                 op = p;
-                crit = crit[p.len()..].trim().to_string();
+                criteria = criteria[p.len()..].trim().to_string();
                 break;
             }
         }
 
-        // Try to parse numeric RHS
+        // Is it a number?
-        let rhs_num = crit.parse::<f64>().ok();
+        let rhs_num = criteria.parse::<f64>().ok();
+
+        // Is it a date?
+        // FIXME: We should parse dates according to locale settings
+        let rhs_date = criteria.parse::<chrono::NaiveDate>().ok();
 
         match op {
             ">" | ">=" | "<" | "<=" => {
-                if let Some(t) = rhs_num {
+                if let Some(d) = rhs_date {
+                    // date comparison
+                    let serial = match date_to_serial_number(d.day(), d.month(), d.year()) {
+                        Ok(sn) => sn as f64,
+                        Err(_) => return false,
+                    };
+
+                    if let CalcResult::Number(n) = db_val {
+                        match op {
+                            ">" => *n > serial,
+                            ">=" => *n >= serial,
+                            "<" => *n < serial,
+                            "<=" => *n <= serial,
+                            _ => false,
+                        }
+                    } else {
+                        false
+                    }
+                } else if let Some(t) = rhs_num {
                     // numeric comparison
                     if let CalcResult::Number(n) = db_val {
                         match op {
@@ -421,7 +544,6 @@ impl Model {
                             _ => false,
                         }
                     } else {
-                        // For non-numbers, use compare_values with a number token to emulate Excel ordering
                         let rhs = CalcResult::Number(t);
                         let c = compare_values(db_val, &rhs);
                         match op {
@@ -434,7 +556,7 @@ impl Model {
                     }
                 } else {
                     // string comparison (case-insensitive) using compare_values semantics
-                    let rhs = CalcResult::String(crit.to_lowercase());
+                    let rhs = CalcResult::String(criteria.to_lowercase());
                     let lhs = match db_val {
                         CalcResult::String(s) => CalcResult::String(s.to_lowercase()),
                         x => x.clone(),
@@ -453,8 +575,8 @@ impl Model {
                 // not equal (with wildcard semantics for strings)
                 // If rhs has wildcards and db_val is string, do regex; else use compare_values != 0
                 if let CalcResult::String(s) = db_val {
-                    if crit.contains('*') || crit.contains('?') {
+                    if criteria.contains('*') || criteria.contains('?') {
-                        if let Ok(re) = from_wildcard_to_regex(&crit.to_lowercase(), true) {
+                        if let Ok(re) = from_wildcard_to_regex(&criteria.to_lowercase(), true) {
                             return !result_matches_regex(
                                 &CalcResult::String(s.to_lowercase()),
                                 &re,
@@ -465,7 +587,7 @@ impl Model {
                 let rhs = if let Some(n) = rhs_num {
                     CalcResult::Number(n)
                 } else {
-                    CalcResult::String(crit.to_lowercase())
+                    CalcResult::String(criteria.to_lowercase())
                 };
                 let lhs = match db_val {
                     CalcResult::String(s) => CalcResult::String(s.to_lowercase()),
@@ -485,18 +607,19 @@ impl Model {
                 } else {
                     // textual/boolean equals (case-insensitive), wildcard-enabled for strings
                     if let CalcResult::String(s) = db_val {
-                        if crit.contains('*') || crit.contains('?') {
+                        if criteria.contains('*') || criteria.contains('?') {
-                            if let Ok(re) = from_wildcard_to_regex(&crit.to_lowercase(), true) {
+                            if let Ok(re) = from_wildcard_to_regex(&criteria.to_lowercase(), true) {
                                 return result_matches_regex(
                                     &CalcResult::String(s.to_lowercase()),
                                     &re,
                                 );
                             }
                         }
-                        return s.to_lowercase() == crit.to_lowercase();
+                        // This is weird but we only need to check if "starts with" for equality
+                        return s.to_lowercase().starts_with(&criteria.to_lowercase());
                     }
                     // Fallback: compare_values equality
-                    compare_values(db_val, &CalcResult::String(crit.to_lowercase())) == 0
+                    compare_values(db_val, &CalcResult::String(criteria.to_lowercase())) == 0
                 }
             }
         }
@@ -528,9 +651,18 @@ impl Model {
             Err(e) => return e,
         };
 
+        if db_right.row <= db_left.row {
+            // no data rows
+            return CalcResult::Error {
+                error: Error::VALUE,
+                origin: cell,
+                message: "No data rows in database".to_string(),
+            };
+        }
+
         let mut best: Option<f64> = None;
 
-        let mut row = db_left.row + 1; // skip header
+        let mut row = db_left.row + 1;
         while row <= db_right.row {
             if self.db_row_matches_criteria(db_left, db_right, row, criteria) {
                 let v = self.evaluate_cell(CellReferenceIndex {
@@ -538,15 +670,15 @@ impl Model {
                     row,
                     column: field_col,
                 });
-                if let CalcResult::Number(n) = v {
+                if let CalcResult::Number(value) = v {
-                    if n.is_finite() {
+                    if value.is_finite() {
                         best = Some(match best {
-                            None => n,
+                            None => value,
                             Some(cur) => {
                                 if want_max {
-                                    n.max(cur)
+                                    value.max(cur)
                                 } else {
-                                    n.min(cur)
+                                    value.min(cur)
                                 }
                             }
                         });
@@ -558,11 +690,7 @@ impl Model {
 
         match best {
             Some(v) => CalcResult::Number(v),
-            None => CalcResult::Error {
+            None => CalcResult::Number(0.0),
-                error: Error::VALUE,
-                origin: cell,
-                message: "No numeric values matched criteria".to_string(),
-            },
         }
     }
 }