pepijnve commented on code in PR #25112: URL: https://github.com/apache/datafusion/pull/25112#discussion_r4070274878
########## datafusion-examples/examples/sql_ops/custom_sql_parser.rs: ########## @@ -16,11 +16,16 @@ // under the License. //! This example demonstrates extending the DataFusion SQL parser to support -//! custom DDL statements, specifically `CREATE EXTERNAL CATALOG`. +//! custom DDL statements, specifically `CREATE FOREIGN CATALOG`. +//! +//! Note: DataFusion supports `CREATE EXTERNAL CATALOG` out-of-the-box making use of +//! [`CatalogProviderFactory`](datafusion::catalog::CatalogProviderFactory). This example +//! is a partial reimplementation of the existing functionality to demonstrate how to extend the +//! SQL parser. Review Comment: TBH I wasn't sure how to describe this well. The point I was trying to get across is that this example is only a partial reimplementation of the functionality provided by the built-in 'external' support. There's no delegation to the registered factories in `handle_create_foreign_catalog`, it's a simple hardcoded example only. I think it would actually be better to replace this example with something else that doesn't overlap with built-in functionality, but I took the easy way out for myself mainly due to lack of inspiration for a different (but still simple to implement) custom syntax alternative. -- 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]
