fix: excel options (#2377)

closes #2372

crates/datafusion_ext/src/functions.rs

@@ -329,6 +329,33 @@ impl TryFrom<FuncParamValue> for IdentValue {
     }
 }
 
+impl TryFrom<FuncParamValue> for usize {
+    type Error = ExtensionError;
+
+    fn try_from(value: FuncParamValue) -> Result<Self> {
+        match value {
+            FuncParamValue::Scalar(s) => match s {
+                ScalarValue::Int8(Some(v)) if v >= 0 => Ok(v as usize),
+                ScalarValue::Int16(Some(v)) if v >= 0 => Ok(v as usize),
+                ScalarValue::Int32(Some(v)) if v >= 0 => Ok(v as usize),
+                ScalarValue::Int64(Some(v)) if v >= 0 => Ok(v as usize),
+                ScalarValue::UInt8(Some(v)) => Ok(v as usize),
+                ScalarValue::UInt16(Some(v)) => Ok(v as usize),
+                ScalarValue::UInt32(Some(v)) => Ok(v as usize),
+                ScalarValue::UInt64(Some(v)) => Ok(v as usize),
+                other => Err(ExtensionError::InvalidParamValue {
+                    param: other.to_string(),
+                    expected: "integer",
+                }),
+            },
+            other => Err(ExtensionError::InvalidParamValue {
+                param: other.to_string(),
+                expected: "unsigned integer",
+            }),
+        }
+    }
+}
+
 impl TryFrom<FuncParamValue> for i64 {
     type Error = ExtensionError;
 

crates/sqlbuiltins/src/functions/table/excel.rs

@@ -57,7 +57,7 @@ impl TableFunc for ExcelScan {
         args: Vec<FuncParamValue>,
         mut opts: HashMap<String, FuncParamValue>,
     ) -> Result<Arc<dyn TableProvider>> {
-        let (source_url, options) = table_location_and_opts(ctx, args, &mut opts)?;
+        let (source_url, _) = table_location_and_opts(ctx, args, &mut opts)?;
 
         let url = match source_url {
             DatasourceUrl::File(path) => path,
@@ -70,19 +70,23 @@ impl TableFunc for ExcelScan {
         };
 
         let url = resolve_path(&url)?;
-        let sheet_name = options.inner.get("sheet_name").map(|v| v.as_str());
-        let has_header = options
-            .inner
-            .get("has_header")
-            .and_then(|v| v.as_str().parse::<bool>().ok());
+        let sheet_name: Option<String> = opts
+            .remove("sheet_name")
+            .map(FuncParamValue::try_into)
+            .transpose()?;
 
-        let infer_schema_len = options
-            .inner
-            .get("infer_rows")
-            .and_then(|v| v.parse::<usize>().ok())
+        let has_header: Option<bool> = opts
+            .remove("has_header")
+            .map(FuncParamValue::try_into)
+            .transpose()?;
+
+        let infer_schema_len = opts
+            .remove("infer_rows")
+            .map(FuncParamValue::try_into)
+            .transpose()?
             .unwrap_or(100);
 
-        let table = read_excel_impl(&url, sheet_name, has_header, infer_schema_len)
+        let table = read_excel_impl(&url, sheet_name.as_deref(), has_header, infer_schema_len)
             .await
             .map_err(|e| ExtensionError::Access(Box::new(e)))?;
         Ok(Arc::new(table))

testdata/sqllogictests/xlsx.slt

@@ -9,3 +9,26 @@ select count(*) from read_excel(
   infer_rows => 10
 )
 
+
+# https://github.com/GlareDB/glaredb/issues/2372
+# make sure multiple sheets work
+statement ok
+select "Resources", "Cost", "Revenue" from read_excel(
+  'file://${PWD}/testdata/xlsx/multiple_sheets.xlsx',
+  has_header => true
+)
+
+
+# https://github.com/GlareDB/glaredb/issues/2372
+query T
+select "HEADING" from read_excel('file://${PWD}/testdata/xlsx/multiple_sheets.xlsx', sheet_name => 'other', has_header => true)
+----
+1
+2
+3
+
+# negatives are not allowed for infer_rows
+statement error
+select * from read_excel('file://${PWD}/testdata/xlsx/multiple_sheets.xlsx', sheet_name => 'other', infer_rows => -1);
+
+