Remove unnecessary filter looking for temp tables#8716
Conversation
|
hi - what about #7168 ? this seems to remove the code that fixed this issue. Does the test in still pass? |
OK it seems this was addressed by some other changes and the issue is fine. OK so this patch if it fixes your problem for azure is likely good for 1.4 |
sqla-tester
left a comment
sqla-tester
left a comment
There was a problem hiding this comment.
OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision f21772a of this pull request into gerrit so we can run tests and reviews and stuff
|
New Gerrit review created for change f21772a: https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
That change would have never been necessary if the solution here was used in the first place. OBJECT_ID is the correct way to check for the existence of a temp table and it already does session differentiation per my MSFT SQL MVP and the documentation I found online. All of the examples use temp..#blah instead of temp.dbo.#blah, and I don't understand the difference honestly, but it seems to fix my issue and the tempdb space is global. |
sqla-tester
left a comment
sqla-tester
left a comment
There was a problem hiding this comment.
OK, this is sqla-tester setting up my work on behalf of CaselIT to try to get revision e2ac7a5 of this pull request into gerrit so we can run tests and reviews and stuff
|
Patchset e2ac7a5 added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
Federico Caselli (CaselIT) wrote: Interesting. Are pg an mysql broken too? View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
mike bayer (zzzeek) wrote: it's got to be my test, or maybe the xdist factor. these passed locally View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
mike bayer (zzzeek) wrote: can't reproduce, no issue. boy I hate temp tables $ pytest test/dialect/test_suite.py -k HasTable --dburi postgresql://scott:tiger@pg14/test -n2 --dbdriver psycopg --dbdriver pg8000 --dbdriver psycopg test/dialect/test_suite.py::HasTableTest_postgresql+pg8000_14_0::test_has_table_cache ========================================================================= 21 passed in 5.81s ============================================================ View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
mike bayer (zzzeek) wrote: oh. duh. it's the connection pool. these have to stick on the same connection View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
mike bayer (zzzeek) wrote: recheck View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
Federico Caselli (CaselIT) wrote: oh indeed View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
mike bayer (zzzeek) wrote: recheck View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
4 similar comments
|
mike bayer (zzzeek) wrote: recheck View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
mike bayer (zzzeek) wrote: recheck View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
mike bayer (zzzeek) wrote: recheck View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
mike bayer (zzzeek) wrote: recheck View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
mike bayer (zzzeek) wrote: alembic recheck View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 |
|
mike bayer (zzzeek) wrote: alembic recheck View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4160 |
|
mike bayer (zzzeek) wrote: alembic-recheck View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4160 |
|
Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4158 has been merged. Congratulations! :) |
|
Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/4160 has been merged. Congratulations! :) |
Fixed issue with :meth:`.Inspector.has_table` when used against a temporary table for the SQL Server dialect would fail an invalid object name error on some Azure variants, due to an unnecessary information schema query that is not supported on those server versions. Pull request courtesy Mike Barry. the patch also fills out test support for has_table() against temp tables, temp views, adding to the has_table() support just added for views in #8700. Fixes: #8714 Closes: #8716 Pull-request: #8716 Pull-request-sha: e2ac7a5 Change-Id: Ia73e4e9e977a2d6b7e100abd2f81a8c8777dc9bb (cherry picked from commit 2af33d79eddc696c0fb1ef749999fa5d0d33f214)
4 participants
Description
Remove code from has_table that doesn't do anything. It was originally added as an attempt to fix: #5597. The correct solution was eventually added for: #6910 but the incorrect solution was not removed even though it could be.
This code is not only inefficient but breaks Azure Synapse Analytics dedicated pools as documented here: #8714
This is documented here: https://learn.microsoft.com/en-us/sql/t-sql/functions/object-id-transact-sql?view=sql-server-ver16#remarks
Fixes: #8714
Checklist
This pull request is:
must include a complete example of the issue. one line code fixes without an
issue and demonstration will not be accepted.
Fixes: #<issue number>in the commit messageinclude a complete example of how the feature would look.
Fixes: #<issue number>in the commit messageHave a nice day!