pepijnve commented on PR #25112: URL: https://github.com/apache/datafusion/pull/25112#issuecomment-5814867009
> So, `CREATE DATABASE` will be supported as an alias/synonym to `CREATE CATALOG` but `CREATE EXTERNAL DATABASE` will not be supported ? That's what https://github.com/apache/datafusion/pull/25112/commits/795f0de5bb57d511a2814b64cb7037f9f3b3dc17 does indeed. I've biased everything towards `CREATE CATALOG`, but retained `CREATE DATABASE` support and added the test you suggested. One thing I'm not entirely sure about in that commit is the method name `CatalogProvider::prepare_deregister_catalog`. I wanted to be able to have a difference in behaviour when dropping an internal catalog vs dropping an external one. `DROP CATALOG <internal>` will error if the catalog is not empty. `DROP CATALOG <external>` should simply detach the catalog. Each implementation of `CatalogProvider` can choose what it does using this callback. The alternative would be to make a distinction at registration time. In other words, we add an additional method `CatalogProviderList::register_external_catalog` so that `CatalogProviderList::deregister_catalog` implementations can decide what to do. I think this would still require a callback on `CatalogProvider` though since you want to be able to make the `DROP CATALOG ... CASCADE` operation an atomic operation which is going to be impossible to do externally. -- 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]
