gabotechs commented on code in PR #45:
URL: https://github.com/apache/datafusion-iceberg/pull/45#discussion_r4310880961
##########
crates/datafusion/src/schema.rs:
##########
@@ -173,38 +175,28 @@ impl SchemaProvider for IcebergSchemaProvider {
let tables = self.tables.clone();
let name_clone = name.clone();
- // Use tokio's spawn_blocking to handle the async work on a blocking
thread pool
- let result = tokio::task::spawn_blocking(move || {
- // Create a new runtime handle to execute the async work
- let rt = tokio::runtime::Handle::current();
- rt.block_on(async move {
- // Verify the input table is empty - CREATE TABLE only accepts
schema definition
- ensure_table_is_empty(&table).await?;
-
- catalog
- .create_table(&namespace, table_creation)
- .await
- .map_err(to_datafusion_error)?;
-
- // Create a new table provider using the catalog reference
- let table_provider = IcebergTableProvider::try_new(
- catalog.clone(),
- namespace.clone(),
- name_clone.clone(),
- )
- .await?;
+ block_on_catalog("create-table", async move {
Review Comment:
DataFusion's `register_table` is synchronous on purpose, because it's not
expecting any IO to happen there. Any solution that passes through issuing an
async blocking call inside synchronous code will be just a workaround that we
must remove as soon as possible.
Ensuring that no IO happens during the `register_table` and
`deregister_table` will require some larger restructure, but it'll be worth it
and essential in the long run, so rather than doubling down on this approach,
I'd be in favor of not merging this PR and instead work an a wider restructure
so that no async code runs inside the methods supposed to by synchronous.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
