alamb commented on code in PR #16502: URL: https://github.com/apache/datafusion/pull/16502#discussion_r2167502727
########## datafusion-cli/src/exec.rs: ########## @@ -231,10 +231,24 @@ pub(super) async fn exec_and_print( let adjusted = AdjustedPrintOptions::new(print_options.clone()).with_statement(&statement); - let plan = create_plan(ctx, statement).await?; + // Only clone the statement if it's a CreateExternalTable Review Comment: This logic is pretty specific to S3 and `create_table` -- inlining it here obscures the rest of the control flow that is happening. Is there any way we can encapsulate these special cases somewhere? Perhaps something like ```rust struct StatementExecutor { statement: DFStatement ... // should we automatically look up the region of a failed s3 request? retry_region: bool, } impl StatementExecutor { async fn execute(&mut self) {} } ``` Then this loop could look something like ```for statement in statements { StatementExecutor::new(ctx, print_options).await?; } ``` ? ########## datafusion-cli/src/exec.rs: ########## @@ -231,10 +231,24 @@ pub(super) async fn exec_and_print( let adjusted = AdjustedPrintOptions::new(print_options.clone()).with_statement(&statement); - let plan = create_plan(ctx, statement).await?; + // Only clone the statement if it's a CreateExternalTable Review Comment: I think then we would have a place to put `retry-region` ########## datafusion-cli/src/object_storage.rs: ########## @@ -32,15 +32,28 @@ use aws_config::BehaviorVersion; use aws_credential_types::provider::error::CredentialsError; use aws_credential_types::provider::{ProvideCredentials, SharedCredentialsProvider}; use log::debug; -use object_store::aws::{AmazonS3Builder, AwsCredential}; +use object_store::aws::{AmazonS3Builder, AmazonS3ConfigKey, AwsCredential}; use object_store::gcp::GoogleCloudStorageBuilder; use object_store::http::HttpBuilder; use object_store::{ClientOptions, CredentialProvider, ObjectStore}; use url::Url; +#[cfg(not(test))] +use object_store::aws::resolve_bucket_region; Review Comment: 👍 -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org