Merged
Conversation
Apparently in SQLAnywhere, you can have a table that has no primary keys, and when it tries to load a table with no primary keys (like through reflection), then in SQLAnyDialectget_pk_constraint(), it threw an uncaught TypeError because in the first sql statement, it tries to get the primary keys, which returns None, and then in the second sql statement, it tries to use the nonexistent result. This commit adds a new Exception class, SQLAnyNoPrimaryKeyError, which is thrown if this condition arises, because it makes no sense to be using something like sqlalchemy with tables that have no primary keys. I'm not sure if i'm supposed to be using sqlalchemy.exec.DBAPIError, but there is no documentation or examples on when or how that is thrown, and through the code, it seems that the Engine takes care of that, but its not expecting a reflection call to fail, so its not doing a "try except Exception: raise DBAPIError()" like it does in other places.
Before, the magical property "supports_native_decimal" was not present in the Dialect class, so sqlalchemy would load the dialect, and see that the property is not present, so its assuming the database is returning Numeric/Decimal types as a python float. However, looking in the python sqlanydb.py module which this uses, it seems the C library that sqlanydb is interfacing with only returns values as Strings, Binary, Doubles, and Integer types, and any other type is probably returned as a string. So what would happen is that sqlalchemy is expecting a float from the database, but sqlanydb was giving it a string. As a result, you would get a nasty warning saying "Dialect sqlany does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur.", followed by an exception saying "a float is required". This commit simply uses the (undocumented...) sqlanydb function to register converters for values returned by the database. I also moved the sqlanydb import to the top of the module, as you can't register the converters till the module is imported, and no reason to delay the import since if you are using this, then you are going to have to load the module eventually...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
4 fixes:
1: added a missing mapping for 'long binary' to the proper SQLAlchemy type
2: we now throw an exception when we try to load/reflect a table that has no primary keys rather then having a confusing "TypeError: none is not subscriptable"
3: Make it so the Dialect supports Decimal objects natively, so we avoid the scary warning from SQLAlchemy about not supporting them natively, and then having an exception "float is required" because the wrapper only returns strings for the Numeric/Decimal type, and SQLAlchemy needs a float or Decimal object.
Dec 4 2014:
Found another bug, since this wasn't pulled yet i'll add it here
4: Added support to pass in SQLAnywhere specific connection arguments using the URL class,
it wasn't returning in the params in URL.query in
SQLAnyDialect.create_connect_args(), so passing anything in for URL(query=???) didn't do anything. Now you can pass in a dictionary into the query kwarg in the URL constructor and have it be passed to the sqlany api correctly.