villebro commented on code in PR #31580:
URL: https://github.com/apache/superset/pull/31580#discussion_r1895982630


##########
superset/db_engine_specs/doris.py:
##########
@@ -245,17 +248,45 @@ def adjust_engine_params(
         catalog: Optional[str] = None,
         schema: Optional[str] = None,
     ) -> tuple[URL, dict[str, Any]]:
-        database = uri.database
-        if schema and database:
-            schema = parse.quote(schema, safe="")
-            if "." in database:
-                database = database.split(".")[0] + "." + schema
-            else:
-                database = "internal." + schema
-            uri = uri.set(database=database)
-
+        if uri.database and "." in uri.database:
+            current_catalog, _ = uri.database.split(".", 1)
+        else:
+            current_catalog = "internal"
+
+        # In Apache Doris, each catalog has an information_schema for BI tool
+        # compatibility. See: https://github.com/apache/doris/pull/28919
+        adjusted_database = ".".join(
+            [catalog or current_catalog or "", "information_schema"]
+        ).rstrip(".")

Review Comment:
   This may sound like a total nit, but I actually had some issues following 
what's going on here, especially the `catalog or current_catalog or ""` logic. 
As `current_catalog` is unnecessary if `catalog` is defined, I would have maybe 
just reused the latter variable for all these uses. Something like:
   
   ```python
           if catalog:
               pass
           elif uri.database and "." in uri.database:
               catalog, _ = uri.database.split(".", 1) or ""  # notice how I 
also moved the `or ""` part here
           else:
               catalog = "internal"
   ```
   Then later just
   
   ```python
           adjusted_database = ".".join([catalog, "information_schema"])
   ```
   Also, why is `.rstrip(".")` needed? I don't see how we can ever hit that, as 
`adjusted_database` will always end with `.information_schema`, right?



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