Allow user_sql_files to override dbcatalog.sql

[Joeboy]

Hi!

For a couple of reasons, the sql in sql/dbcatalog.sql is causing us problems:

1. pg_temp doesn't work on AWS Aurora
2. Our Aurora database is unfortunately still on postgress 11, and the sql fails with an error there

It looks like we could nearly work around those issues by passing custom user_sql_files into connect_to_db(). But, we still have the problem of dbcatalog.sql erroring out. It seems that the only way to prevent that is to override connect_to_db() and connection_factory, removing the lines that run the sql from the latter class. Which seems quite messy.

Maybe it'd be neater if user_sql_files ran instead of dbcatalog.sql, not before it? If somebody just wants the default + their custom sql, it's easy enough to include tipg.database.DB_CATALOG_FILE in the list. It'd also make it possible to choose the order the files run in. Or to skip inspecting the db and create the collection catalog in some other way.

What do you think? If I did a PR what'd be the chances it'd be accepted?

[vincentsarago]

Dear @Joeboy, thanks for starting this discussion.

as mentioned in previous threads (see #85), Aurora is not Postgres (but Postgres compatible, while with TiPG we focused on Postgres). We're open to support Aurora but not at all cost.

I think it will be pretty easy for you to write your own application using tipg (example https://github.com/developmentseed/tipgstac), You'll almost just need to have a custom connect_to_db, connection_factory and register_collection_catalog

[vincentsarago]

I've started something over https://github.com/vincentsarago/tipg-aurora (but nothing is tested 😅 )

[Joeboy]

Hi! Thanks for your very quick reply. Just to check we're on the same page - what I'm suggesting would add a net one line to the TiPg code:

--- a/tipg/database.py
+++ b/tipg/database.py
@@ -33,7 +33,11 @@ class connection_factory:
     ) -> None:
         """Init."""
         self.schemas = schemas or []
-        self.user_sql_files = user_sql_files or []
+        if user_sql_files is None:
+            # Register TiPG functions in `pg_temp`
+            self.user_sql_files = [DB_CATALOG_FILE]
+        else:
+            self.user_sql_files = user_sql_files
 
     async def __call__(self, conn: asyncpg.Connection):
         """Create connection."""
@@ -63,9 +67,6 @@ class connection_factory:
         for sqlfile in self.user_sql_files:
             await conn.execute(sqlfile.read_text())
 
-        # Register TiPG functions in `pg_temp`
-        await conn.execute(DB_CATALOG_FILE.read_text())
-

And that would make it possible to customize (or skip) the startup sql with about 60 fewer lines. To me, that seems like quite a good trade-off. I'll of course respect your decision if you still disagree, just wanted to check I'd explained myself properly.

[vincentsarago]

This change would be a bit dangerous and will break current workflow.

Tipg is build around the function present in the DB_CATALOG_FILE which is why we execute it in another step. The user_sql_files should host the custom functions/layers but it's true that in theory you could have a custom CATALOG_FILE within the list.

With the proposed change you will only have 1 part of the changes needed to get rid of pg_temp , you'll still need to write a custom get_collection_index/register_collection_catalog method 🤷‍♂️