Strip quotes from schemas and tables prior to writing them to catalog

datafusion_remote_tables/src/factory.rs

@@ -22,9 +22,9 @@ impl TableProviderFactory for RemoteTableFactory {
         let table = RemoteTable::new(
             cmd.options
                 .get("name")
-                .ok_or(DataFusionError::Execution(
-                    "Missing 'name' option".to_string(),
-                ))?
+                .ok_or_else(|| {
+                    DataFusionError::Execution("Missing 'name' option".to_string())
+                })?
                 .clone(),
             cmd.location.clone(),
             SchemaRef::from(cmd.schema.deref().clone()),

src/config/context.rs

@@ -89,7 +89,7 @@ fn build_object_store(cfg: &schema::SeafowlConfig) -> Arc<dyn ObjectStore> {
             let mut builder = AmazonS3Builder::new()
                 .with_access_key_id(access_key_id)
                 .with_secret_access_key(secret_access_key)
-                .with_region(region.clone().unwrap_or("".to_string()))
+                .with_region(region.clone().unwrap_or_default())
                 .with_bucket_name(bucket)
                 .with_allow_http(true);
 

src/repository/default.rs

@@ -164,8 +164,9 @@ impl Repository for $repo {
     }
 
     async fn create_database(&self, database_name: &str) -> Result<DatabaseId, Error> {
+        let unqoted_name = database_name.trim_matches('"');
         let id = sqlx::query(r#"INSERT INTO database (name) VALUES ($1) RETURNING (id)"#)
-            .bind(database_name)
+            .bind(unqoted_name)
             .fetch_one(&self.executor)
             .await.map_err($repo::interpret_error)?
             .try_get("id").map_err($repo::interpret_error)?;
@@ -212,9 +213,10 @@ impl Repository for $repo {
         database_id: DatabaseId,
         collection_name: &str,
     ) -> Result<CollectionId, Error> {
+        let unquoted_name = collection_name.trim_matches('"');
         let id = sqlx::query(
             r#"INSERT INTO "collection" (database_id, name) VALUES ($1, $2) RETURNING (id)"#,
-        ).bind(database_id).bind(collection_name)
+        ).bind(database_id).bind(unquoted_name)
         .fetch_one(&self.executor)
         .await.map_err($repo::interpret_error)?
         .try_get("id").map_err($repo::interpret_error)?;
@@ -229,11 +231,12 @@ impl Repository for $repo {
         schema: &Schema,
     ) -> Result<(TableId, TableVersionId), Error> {
         // Create new (empty) table
+        let unquoted_name = table_name.trim_matches('"');
         let new_table_id: i64 = sqlx::query(
             r#"INSERT INTO "table" (collection_id, name) VALUES ($1, $2) RETURNING (id)"#,
         )
         .bind(collection_id)
-        .bind(table_name)
+        .bind(unquoted_name)
         .fetch_one(&self.executor)
         .await.map_err($repo::interpret_error)?
         .try_get("id").map_err($repo::interpret_error)?;
@@ -515,14 +518,15 @@ impl Repository for $repo {
         details: &CreateFunctionDetails,
     ) -> Result<FunctionId, Error> {
         let input_types = serde_json::to_string(&details.input_types).expect("Couldn't serialize input types!");
+        let unquoted_name = function_name.trim_matches('"');
 
         let new_function_id: i64 = sqlx::query(
             r#"
         INSERT INTO "function" (database_id, name, entrypoint, language, input_types, return_type, data, volatility)
         VALUES ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING (id);
         "#)
             .bind(database_id)
-            .bind(function_name)
+            .bind(unquoted_name)
             .bind(details.entrypoint.clone())
             .bind(details.language.to_string())
             .bind(input_types)

src/repository/interface.rs

@@ -242,15 +242,19 @@ pub mod tests {
         }
     }
 
-    async fn make_database_with_single_table(
+    // bumped into rustc bug: https://github.com/rust-lang/rust/issues/96771#issuecomment-1119886703
+    async fn make_database_with_single_table<'a>(
         repository: Arc<dyn Repository>,
+        database_name: &'a str,
+        collection_name: &'a str,
+        table_name: &'a str,
     ) -> (DatabaseId, CollectionId, TableId, TableVersionId) {
         let database_id = repository
-            .create_database("testdb")
+            .create_database(database_name)
             .await
             .expect("Error creating database");
         let collection_id = repository
-            .create_collection(database_id, "testcol")
+            .create_collection(database_id, collection_name)
             .await
             .expect("Error creating collection");
 
@@ -263,7 +267,7 @@ pub mod tests {
         };
 
         let (table_id, table_version_id) = repository
-            .create_table(collection_id, "testtable", &schema)
+            .create_table(collection_id, table_name, &schema)
             .await
             .expect("Error creating table");
 
@@ -279,6 +283,7 @@ pub mod tests {
         test_create_functions(repository.clone(), database_id).await;
         test_rename_table(repository.clone(), database_id, table_id, new_version_id)
             .await;
+        test_create_with_name_in_quotes(repository.clone()).await;
         test_error_propagation(repository, table_id).await;
     }
 
@@ -296,20 +301,21 @@ pub mod tests {
         version: TableVersionId,
         collection_name: String,
         table_name: String,
+        table_id: i64,
     ) -> Vec<AllDatabaseColumnsResult> {
         vec![
             AllDatabaseColumnsResult {
                 collection_name: collection_name.clone(),
                 table_name: table_name.clone(),
-                table_id: 1,
+                table_id,
                 table_version_id: version,
                 column_name: "date".to_string(),
                 column_type: "{\"children\":[],\"name\":\"date\",\"nullable\":false,\"type\":{\"name\":\"date\",\"unit\":\"MILLISECOND\"}}".to_string(),
             },
             AllDatabaseColumnsResult {
                 collection_name,
                 table_name,
-                table_id: 1,
+                table_id,
                 table_version_id: version,
                 column_name: "value".to_string(),
                 column_type: "{\"children\":[],\"name\":\"value\",\"nullable\":false,\"type\":{\"name\":\"floatingpoint\",\"precision\":\"DOUBLE\"}}"
@@ -322,7 +328,13 @@ pub mod tests {
         repository: Arc<dyn Repository>,
     ) -> (DatabaseId, TableId, TableVersionId) {
         let (database_id, _, table_id, table_version_id) =
-            make_database_with_single_table(repository.clone()).await;
+            make_database_with_single_table(
+                repository.clone(),
+                "testdb",
+                "testcol",
+                "testtable",
+            )
+            .await;
 
         // Test loading all columns
 
@@ -333,7 +345,7 @@ pub mod tests {
 
         assert_eq!(
             all_columns,
-            expected(1, "testcol".to_string(), "testtable".to_string())
+            expected(1, "testcol".to_string(), "testtable".to_string(), 1)
         );
 
         // Duplicate the table
@@ -353,7 +365,8 @@ pub mod tests {
             expected(
                 new_version_id,
                 "testcol".to_string(),
-                "testtable".to_string()
+                "testtable".to_string(),
+                1
             )
         );
 
@@ -365,7 +378,7 @@ pub mod tests {
 
         assert_eq!(
             all_columns,
-            expected(1, "testcol".to_string(), "testtable".to_string())
+            expected(1, "testcol".to_string(), "testtable".to_string(), 1)
         );
 
         // Check the existing table versions
@@ -382,6 +395,55 @@ pub mod tests {
         (database_id, table_id, table_version_id)
     }
 
+    async fn test_create_with_name_in_quotes(repository: Arc<dyn Repository>) {
+        let (database_id, _, _, table_version_id) = make_database_with_single_table(
+            repository.clone(),
+            "testdb2",
+            "\"testcol\"",
+            "\"testtable\"",
+        )
+        .await;
+
+        // Test loading all columns
+
+        let all_columns = repository
+            .get_all_columns_in_database(database_id, None)
+            .await
+            .expect("Error getting all columns");
+
+        assert_eq!(
+            all_columns,
+            expected(
+                table_version_id,
+                "testcol".to_string(),
+                "testtable".to_string(),
+                2
+            )
+        );
+
+        // Duplicate the table
+        let new_version_id = repository
+            .create_new_table_version(table_version_id, true)
+            .await
+            .unwrap();
+
+        // Test all columns again: we should have the schema for the latest table version
+        let all_columns = repository
+            .get_all_columns_in_database(database_id, None)
+            .await
+            .expect("Error getting all columns");
+
+        assert_eq!(
+            all_columns,
+            expected(
+                new_version_id,
+                "testcol".to_string(),
+                "testtable".to_string(),
+                2
+            )
+        );
+    }
+
     async fn test_create_append_partition(
         repository: Arc<dyn Repository>,
         table_version_id: TableVersionId,
@@ -529,7 +591,8 @@ pub mod tests {
             expected(
                 table_version_id,
                 "testcol".to_string(),
-                "testtable2".to_string()
+                "testtable2".to_string(),
+                1
             )
         );
 
@@ -553,7 +616,8 @@ pub mod tests {
             expected(
                 table_version_id,
                 "testcol2".to_string(),
-                "testtable2".to_string()
+                "testtable2".to_string(),
+                1
             )
         );
     }

src/wasm_udf/data_types.rs

@@ -62,28 +62,22 @@ pub enum CreateFunctionDataType {
 
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, EnumString, Display, Clone)]
 #[serde(rename_all = "camelCase")]
+#[derive(Default)]
 pub enum CreateFunctionVolatility {
     Immutable,
     Stable,
+    #[default]
     Volatile,
 }
-impl Default for CreateFunctionVolatility {
-    fn default() -> Self {
-        CreateFunctionVolatility::Volatile
-    }
-}
 
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, EnumString, Display, Clone)]
 #[serde(rename_all = "camelCase")]
+#[derive(Default)]
 pub enum CreateFunctionLanguage {
+    #[default]
     Wasm,
     WasmMessagePack,
 }
-impl Default for CreateFunctionLanguage {
-    fn default() -> Self {
-        CreateFunctionLanguage::Wasm
-    }
-}
 
 fn parse_create_function_data_type(
     raw: &str,