alamb commented on code in PR #4112:
URL: https://github.com/apache/arrow-datafusion/pull/4112#discussion_r1015855416


##########
datafusion/core/src/execution/context.rs:
##########
@@ -2267,30 +2288,33 @@ mod tests {
 
         let mut table_factories: HashMap<String, Arc<dyn 
TableProviderFactory>> =
             HashMap::new();
-        table_factories.insert("test".to_string(), Arc::new(TestTableFactory 
{}));
+        let factory = Arc::new(ListingTableFactory::new(FileType::CSV));
+        table_factories.insert("test".to_string(), factory);
         let rt_cfg = 
RuntimeConfig::new().with_table_factories(table_factories);
         let runtime = Arc::new(RuntimeEnv::new(rt_cfg).unwrap());
         let cfg = SessionConfig::new()
             .set_str("datafusion.catalog.location", url.as_str())
             .set_str("datafusion.catalog.type", "test");
         let session_state = SessionState::with_config_rt(cfg, runtime);
         let ctx = SessionContext::with_state(session_state);
+        ctx.refresh_catalogs().await?;
 
-        let mut table_count = 0;
-        for cat_name in ctx.catalog_names().iter() {
-            let cat = ctx.catalog(cat_name).unwrap();
-            for s_name in cat.schema_names().iter() {
-                let schema = cat.schema(s_name).unwrap();
-                if let Some(listing) =
-                    schema.as_any().downcast_ref::<ListingSchemaProvider>()
-                {
-                    listing.refresh().await.unwrap();
-                    table_count = schema.table_names().len();
-                }
-            }
-        }
+        let result =
+            plan_and_collect(&ctx, "select c_name from default.customer limit 
3;")
+                .await?;
+
+        let actual = arrow::util::pretty::pretty_format_batches(&result)

Review Comment:
   You might also consider using `assert_batches_eq` here in this test



##########
datafusion/core/src/execution/context.rs:
##########
@@ -175,6 +175,26 @@ impl SessionContext {
         Self::with_config(SessionConfig::new())
     }
 
+    /// Finds any ListSchemaProviders and instructs them to reload tables from 
"disk"
+    pub async fn refresh_catalogs(&self) -> Result<()> {

Review Comment:
   makes sense to me



##########
datafusion/core/src/execution/context.rs:
##########
@@ -175,6 +175,26 @@ impl SessionContext {
         Self::with_config(SessionConfig::new())
     }
 
+    /// Finds any ListSchemaProviders and instructs them to reload tables from 
"disk"

Review Comment:
   ```suggestion
       /// Invokes  `ListingSchemaProvider::reload()` for all registered 
providers
   ```



##########
datafusion-cli/src/object_storage.rs:
##########
@@ -141,7 +141,13 @@ mod tests {
         assert!(err.to_string().contains("Generic S3 error: Missing region"));
 
         env::set_var("AWS_REGION", "us-east-1");
-        assert!(provider.get_by_url(&Url::from_str(s3).unwrap()).is_ok());
+        let url = Url::from_str(s3).expect("Unable to parse s3 url");
+        let res = provider.get_by_url(&url);
+        let msg = match res {
+            Err(e) => format!("{}", e),
+            Ok(_) => "".to_string()
+        };
+        assert_eq!("".to_string(), msg); // Fail with error message

Review Comment:
   Could you please file a ticket to do so? Thank you!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to