Skip to content

Fix(fabric): Ensure that Fabric connections do not try to use a catalog once it's been dropped#5314

Merged
erindru merged 3 commits intomainfrom
erin/fix-fabric
Sep 8, 2025
Merged

Fix(fabric): Ensure that Fabric connections do not try to use a catalog once it's been dropped#5314
erindru merged 3 commits intomainfrom
erin/fix-fabric

Conversation

@erindru
Copy link
Collaborator

@erindru erindru commented Sep 8, 2025

Previously, connecting to Fabric with an invalid / nonexistent database would succeed and it would just default to the first database in the list.

It appears that Microsoft tightened this behavior up recently, causing errors like this on main:

pyodbc.InterfaceError: ('28000', "[28000] [Microsoft][ODBC Driver 18 for SQL Server][SQL Server]Login failed for user '<token-identified principal>'. (18456) (SQLDriverConnect)")

This PR calls self.close() on the EngineAdapter after dropping a catalog to trigger the following behaviour:

  • Fresh connections get established so any connections pointing to the dropped database are re-created
  • In order for this to work, threadlocal data needs to be cleared to ensure that when connections are re-established, they use the default catalog and not the value of the target_catalog attribute which may point to the catalog that just got dropped

It's a fairly heavy-handed approach, ideally we would selectively close connections that were pointed to the catalog that just got dropped but that was a lot more effort for minimal return because Fabric is not heavily used. If it turns into an issue, it could be revisited

only:
- main
#- gcp-postgres
#filters:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: revert

self._discard_transaction(thread_id)
self._thread_attributes.pop(thread_id, None)

self._thread_attributes.clear()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This threw me for a while.

You can set threadlocal data without a connection, but it will only be cleared if a connection was created. This is a problem because calling close_all() and then opening a new connection will pick up old values.

I changed it so that close_all() clears all threadlocal data regardless and then added a test test_thread_local_connection_pool_attributes to prove it's working

@erindru erindru enabled auto-merge (squash) September 8, 2025 20:22
@erindru erindru merged commit 89d5740 into main Sep 8, 2025
36 checks passed
@erindru erindru deleted the erin/fix-fabric branch September 8, 2025 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants