Ujjwaljain16 commented on code in PR #43974:
URL: https://github.com/apache/superset/pull/43974#discussion_r4023008242
##########
superset/db_engine_specs/mssql.py:
##########
@@ -194,6 +195,82 @@ def fetch_data(
# Lists of `pyodbc.Row` need to be unpacked further
return cls.pyodbc_rows_to_tuples(data)
+ @classmethod
+ def get_catalog_from_engine_params(
+ cls,
+ sqlalchemy_uri: URL,
+ connect_args: dict[str, Any],
+ ) -> str | None:
+ """
+ Resolve the database from genuine, statically-configured connection
+ settings only: an explicit ``connect_args["database"]``, a
+ ``Database=``/``Initial Catalog=`` entry embedded in the documented
+ ``odbc_connect`` connection-string query parameter, or the URL's own
+ database segment.
+
+ The check order mirrors ``MSDialect_pyodbc.create_connect_args``'s
+ actual precedence, not just plausibility:
+
+ - ``connect_args`` is always appended, as extra keyword arguments, to
+ whatever connection string SQLAlchemy hands to ``pyodbc.connect()``
+ -- regardless of whether that string came from the URL or from
+ ``odbc_connect`` -- so a duplicate key there wins over both.
+ - When ``odbc_connect`` is present, SQLAlchemy uses it as the *entire*
+ connection string and never looks at the URL's host/database
+ segments at all, so it must be checked before falling back to
+ ``sqlalchemy_uri.database``.
+
+ Returns None when none of these statically state a database -- e.g. a
+ host/DSN-only URI that relies on the SQL login's server-side default
+ database. That default is only known to SQL Server itself, at connect
+ time; resolving it would require a live query, which this method
+ deliberately does not perform.
+ """
+ if isinstance(database := connect_args.get("database"), str) and
database:
Review Comment:
@gabotorresruiz
one additional detail came up while pinning down the precedence:
SQLAlchemy also accepts `database` as a URL query parameter, and
`PyODBCConnector` merges `url.query` after the path-derived options so a
conflicting `?database=query_db` actually overrides a path-segment
`database=path_db` in the connection string
i included that in the same precedence fix, along with coverage for the
query-param-only, query-param vs `connect_args`, query-param vs path and empty
query-param cases
everything is ready for another look....
--
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]
