sungwy commented on issue #1148: URL: https://github.com/apache/iceberg-python/issues/1148#issuecomment-2338726996
Hi @isc-patrick - thank you for raising this. These are great points. I'm wondering if we can keep the current approach of running `_ensure_table_exists` within the instantiation of the SqlCatalog, but adopt an improved way of running the table creation DDL as you suggested in (2). The reason is, because I think as long as the function is idempotent (similar to `create_table_if_not_exists` or `CREATE TABLE IF NOT EXISTS`), it's simple enough to keep in the initialization of the SqlCatalog, which requires these tables to execute any of its member functions. Looking at the SqlAlchemy dialect further, I'm of the impression that we could instead just remove the additional try exception handling and just invoke `SqlCatalogBaseTable.metadata.create_all(self.engine)`. According to the SqlAlchemy docs, this uses the default flag of `checkfirst=True`, which avoids creating the tables if they already exist. https://docs.sqlalchemy.org/en/20/core/metadata.html#sqlalchemy.schema.MetaData.create_all What do you think about this suggestion? -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
