Skip to content
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

MetaData reflect is using schema name instead of db name and causing error in SQL Server #4923

Closed
Feulo opened this issue Oct 17, 2019 · 16 comments
Labels
bug Something isn't working high priority SQL Server Microsoft SQL Server, e.g. mssql
Milestone

Comments

@Feulo
Copy link

Feulo commented Oct 17, 2019

I'm currently working in a SQL SERVER database, over a network, named "fit_alunos" that contains a lot of schemas with the pattern "SALAS{first name}.{last name}"
I can create the engine and connect to the database without problem:

from sqlalchemy import create_engine, inspect

eng = create_engine('mssql+pymssql://salas\guilherme.santo:pass@server/fit_alunos?charset=utf8')

then if I inspect the engine, everything seems ok:

insp = inspect(engine)

insp.default_schema_name  # 'SALAS\\Guilherme.Santo'
insp.get_schema_names()  # a list of schemas with the pattern SALAS\\'something'
insp.get_table_names()  # all the tables in my schema, with no problem

but if I try to create a MetaData object and reflect the engine:

from sqlachemy import MetaData

meta = MetaData()
meta.reflect(bind=eng)

I got this OperationalError:

OperationalError: (pymssql.OperationalError) (911, b"Database 'SALAS\\Guilherme' does not exist. Make sure that the name is entered correctly.DB-Lib error message 20018, severity 16:\nGeneral SQL Server error: Check messages from the SQL Server\n")
[SQL: use [SALAS\Guilherme]]
(Background on this error at: http://sqlalche.me/e/e3q8)

I guess that SQLAlchemy is interpreting that the database is "SALAS\Guilherme" and the schema is "Santo", instead of the the database "fit_alunos" and the schema "SALAS\Guilherme.Santo".

So, I ran the reflect method wit an engine with echo=True and find that it gets the db name using a SQL function:

2019-10-17 16:27:16,330 INFO sqlalchemy.engine.base.Engine select db_name()
2019-10-17 16:27:16,330 INFO sqlalchemy.engine.base.Engine {}
2019-10-17 16:27:16,350 INFO sqlalchemy.engine.base.Engine use [SALAS\Guilherme]
2019-10-17 16:27:16,350 INFO sqlalchemy.engine.base.Engine {}
2019-10-17 16:27:16,389 INFO sqlalchemy.engine.base.Engine ROLLBACK
---------------------------------------------------------------------------
MSSQLDatabaseException                    Traceback (most recent call last)

It seems that SELECT db_name() is returning the schema name instead of the db name.

But then, I tested the return value from the SQL functions that get the DB name and schema name, and it seems to be right

with eng.connect() as con: 
     rs = con.execute("select schema_name();") 
     print(rs.fetchall())  # [('SALAS\\Guilherme.Santo',)]
     rs = con.execute("select db_name();") 
     print(rs.fetchall())  # [('fit_alunos',)]

Any idea of what can be happening?

@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2019

can you try sqlalchemy 1.3.8 please assuming this is 1.3.9, this would indicate a regression caused by #4883 . although this looks like this problem might pre-date that change overall. the "use" you see is that it is attempting to swtich the database from fit_alunos to Guilherme.Santo.

However, I don't see why it would do any of that if you used MetaData().refect() and did not pass any kind of schema argument. Can you show the exact code you're running and the full stack trace for the error please.

@zzzeek zzzeek added bug Something isn't working SQL Server Microsoft SQL Server, e.g. mssql labels Oct 17, 2019
@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2019

do any of the tables in the default schema have foreign key references to tables in the named schemas ? that is, is there cross-schema foreign key reflection going on ?

@Feulo
Copy link
Author

Feulo commented Oct 17, 2019

Hey,

I'm using SQLAlchemy 1.3.10, I can try to downgrade ant test again later.

the code I used is exactly the code in the post, but here it is: (I just use getpass to get the password)
reflect_test.txt

the complete output for the code is here:
output.txt

I just realized that the problem is not only with the reflection method if try:

from sqlachemy import create_engine
eng = create_engine("mssql+pymssql://salas\guilherme.santo:{getpass()}@200.49.54.234/fit_alunos?charset=utf8")

eng.table_names()

I got the same problem with this output:
output_table_names.txt

@Feulo
Copy link
Author

Feulo commented Oct 17, 2019

do any of the tables in the default schema have foreign key references to tables in the named schemas ? that is, is there cross-schema foreign key reflection going on ?

All the tables are very simple, I am using just to get used to SQLAlchemy, only one table has an FK but is to an table inside the same schema ("SALAS\Guilherme.santo")

@Feulo
Copy link
Author

Feulo commented Oct 17, 2019

I just tested with SQLAlchemy 1.3.8 and got the same result.
I Also tested with someonelse's username and password and it worked! the problem seems to be the fact that my schema is named after my user name "guilherme.santo", and the dot beetween my first and last names breaks something. I tested with a user that does not have a dot in it (and therefore the schema either) and everything worked.

@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2019

it's because of a convention SQLAlchemy has had for many years that assumes there are no dots in a schema name, and instead when it sees a dot in a schema name it assumes that this is actually a [databasename.schemaname] combination, which it splits up. in this case the default schema name itself has a dot in it which seems to be blowing things up. metadata.reflect() should not be getting anything from all of these other schemas because it was not passed any schema name.

I have no solution at the moment, other than patching the code for your specific case which will break everyone elses', and will have to spend a few days trying to think of one.

@zzzeek zzzeek added this to the 1.3.xx milestone Oct 17, 2019
@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2019

here's a patch that I would have to recreate your environent in order to test and I haven't tried this yet. can you try this out? it will apply quoting (brackets) to the names received from the db_name and schema_name functions.

diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index d01acab12..061f9170e 100644
--- a/lib/sqlalchemy/dialects/mssql/base.py
+++ b/lib/sqlalchemy/dialects/mssql/base.py
@@ -2150,9 +2150,15 @@ def _db_plus_owner(fn):
             dialect,
             connection,
             tablename,
-            dbname,
-            owner,
-            schema,
+            connection.dialect.identifier_preparer.quote_schema(dbname)
+            if dbname
+            else None,
+            connection.dialect.identifier_preparer.quote_schema(owner)
+            if owner
+            else None,
+            connection.dialect.identifier_preparer.quote_schema(schema)
+            if schema
+            else None,
             **kw
         )

@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2019

that's probably not going to work.

@zzzeek
Copy link
Member

zzzeek commented Oct 17, 2019

here's another approach that might work. i think this might also be different in 1.4.

diff --git a/lib/sqlalchemy/dialects/mssql/base.py b/lib/sqlalchemy/dialects/mssql/base.py
index d01acab12..89e718702 100644
--- a/lib/sqlalchemy/dialects/mssql/base.py
+++ b/lib/sqlalchemy/dialects/mssql/base.py
@@ -2399,7 +2399,7 @@ class MSDialect(default.DefaultDialect):
             query = sql.text("SELECT schema_name()")
             default_schema_name = connection.scalar(query)
             if default_schema_name is not None:
-                return util.text_type(default_schema_name)
+                return quoted_name(default_schema_name, quote=True)
             else:
                 return self.schema_name
 

@Feulo
Copy link
Author

Feulo commented Oct 18, 2019

Thanks, I imagined that problem could be the schema's name. Unfortunelly I have no control over the schema's names. Thank you for the patch I'll try it as soon as possible and give a feedback.

@zzzeek
Copy link
Member

zzzeek commented Oct 18, 2019

the second patch seems to fix it, however the issue also does not occur in master / 1.4 where i had done some refactoring of some of this code, seeing if i can leverage that aspect of it.

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the master branch:

Ensure SQL Server default schema name not interpreted as dot-separated tokens https://gerrit.sqlalchemy.org/1525

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the rel_1_3 branch:

Ensure SQL Server default schema name not interpreted as dot-separated tokens https://gerrit.sqlalchemy.org/1526

@zzzeek
Copy link
Member

zzzeek commented Oct 18, 2019

I would not be surprised if sqlalchemy has more issues with this naming convention, however the immediate issue should be fixed by that patch going through. if you can try it out and then do a little more with Core or ORM maybe we can find some more issues to fix.

@Feulo
Copy link
Author

Feulo commented Oct 19, 2019

Hey,

I've tested some commands with the core language and the ORM and, until now, it seems to be working just fine, thanks again for the patch.

if I found another bug related to the schema name I'll post it here.

@zzzeek
Copy link
Member

zzzeek commented Oct 19, 2019

OK we'll commit that and see how it goes.

when you use these schema names, you need to put brackets around them, the mssql dialect in SQLAlchemy will then know to not split out on the dot to get the "owner" / "database" from it. it's not ideal but that's where we're at. see https://docs.sqlalchemy.org/en/13/dialects/mssql.html#multipart-schema-names

sqlalchemy-bot pushed a commit that referenced this issue Oct 21, 2019
…d tokens

Fixed an issue in the :meth:`.Engine.table_names` method where it would
feed the dialect's default schema name back into the dialect level table
function, which in the case of SQL Server would interpret it as a
dot-tokenized schema name as viewed by the mssql dialect, which would
cause the method to fail in the case where the database username actually
had a dot inside of it.  In 1.3, this method is still used by the
:meth:`.MetaData.reflect` function so is a prominent codepath. In 1.4,
which is the current master development branch, this issue doesn't exist,
both because :meth:`.MetaData.reflect` isn't using this method nor does the
method pass the default schema name explicitly.  The fix nonetheless
guards against the default server name value returned by the dialect from
being interpreted as dot-tokenized name under any circumstances by
wrapping it in quoted_name().

Fixes: #4923
Change-Id: I821bd38ed89b767eaca0bdffee7f8ba3baf82560
(cherry picked from commit f50c6a04067acf2cd2fc5e42d5acaa9206d9a078)
@zzzeek zzzeek modified the milestones: 1.3.xx, 1.3.x Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high priority SQL Server Microsoft SQL Server, e.g. mssql
Projects
None yet
Development

No branches or pull requests

3 participants