-
Notifications
You must be signed in to change notification settings - Fork 14.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Fix regression introduced in #20893 #21743
fix: Fix regression introduced in #20893 #21743
Conversation
Codecov Report
@@ Coverage Diff @@
## master #21743 +/- ##
==========================================
- Coverage 66.84% 65.39% -1.46%
==========================================
Files 1800 1800
Lines 68945 68957 +12
Branches 7335 7335
==========================================
- Hits 46086 45092 -994
- Misses 20966 21972 +1006
Partials 1893 1893
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i assume sqlalchemy filter is AND
by default? I thought you needed to use the _and
function or something usually?
You have CI as your test plan, is there anyway to write a unit test for this? Or can you test locally to ensure this works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah:
Multiple criteria may be specified as comma separated; the effect is that they will be joined together using the and_() function:
from https://docs.sqlalchemy.org/en/14/orm/query.html#sqlalchemy.orm.Query.filter
i'll stamp, but a full test plan would be nice
Thanks for the review @etr2460. I updated the testing instructions per your request. |
SUMMARY
This PR fixes a regression introduced in #20893 where when fetching the extra table metadata the query should be restricted to the database in question given that schema/table names aren't unique across databases.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
CI, i.e., existing integration tests.
ADDITIONAL INFORMATION