codeant-ai-for-open-source[bot] commented on code in PR #44825:
URL: https://github.com/apache/superset/pull/44825#discussion_r4145940902


##########
superset/db_engine_specs/redshift.py:
##########
@@ -253,6 +262,51 @@ class RedshiftEngineSpec(BasicParametersMixin, 
PostgresBaseEngineSpec):
         ),
     }
 
+    @classmethod
+    def adjust_engine_params(
+        cls,
+        uri: URL,
+        connect_args: dict[str, Any],
+        catalog: str | None = None,
+        schema: str | None = None,
+    ) -> tuple[URL, dict[str, Any]]:
+        """
+        Set the catalog (database).
+        """
+        if catalog:
+            uri = uri.set(database=catalog)
+            # IAM connections pass the database in connect_args instead
+            if "database" in connect_args:
+                connect_args["database"] = catalog
+
+        return uri, connect_args
+
+    @classmethod
+    def get_default_catalog(cls, database: Database) -> str | None:
+        """
+        Return the default catalog for a given database.
+        """
+        return database.url_object.database

Review Comment:
   ✅ **CodeAnt verified this suggestion was addressed in subsequent commits and 
marked this thread resolved** as of `65fa56d`.
   
   `get_default_catalog` now falls back to 
`engine_params.connect_args["database"]` when the URL has no database.
   
   <sub>If that's not right, unresolve this thread and CodeAnt will leave it 
open.</sub>
   
   <!-- codeant-auto-resolve-reply -->



##########
superset/db_engine_specs/redshift.py:
##########
@@ -253,6 +262,51 @@ class RedshiftEngineSpec(BasicParametersMixin, 
PostgresBaseEngineSpec):
         ),
     }
 
+    @classmethod
+    def adjust_engine_params(
+        cls,
+        uri: URL,
+        connect_args: dict[str, Any],
+        catalog: str | None = None,
+        schema: str | None = None,
+    ) -> tuple[URL, dict[str, Any]]:
+        """
+        Set the catalog (database).
+        """
+        if catalog:
+            uri = uri.set(database=catalog)
+            # IAM connections pass the database in connect_args instead
+            if "database" in connect_args:
+                connect_args["database"] = catalog
+
+        return uri, connect_args
+
+    @classmethod
+    def get_default_catalog(cls, database: Database) -> str | None:
+        """
+        Return the default catalog for a given database.
+        """
+        return database.url_object.database
+
+    @classmethod
+    def get_catalog_names(
+        cls,
+        database: Database,
+        inspector: Inspector,
+    ) -> set[str]:
+        """
+        Return all catalogs.
+
+        In Redshift, a catalog is called a "database". SVV_REDSHIFT_DATABASES
+        also lists databases created from datashares, which pg_database does 
not.
+        """
+        return {
+            catalog
+            for (catalog,) in inspector.bind.execute(
+                sa.text("SELECT database_name FROM svv_redshift_databases")
+            )
+        }

Review Comment:
   ✅ **CodeAnt verified this suggestion was addressed in subsequent commits and 
marked this thread resolved** as of `65fa56d`.
   
   Catalog discovery now opens a connection with `inspector.engine.connect()` 
and executes the query through the connection rather than calling `execute` on 
the engine.
   
   <sub>If that's not right, unresolve this thread and CodeAnt will leave it 
open.</sub>
   
   <!-- codeant-auto-resolve-reply -->



-- 
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]

Reply via email to