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

Fixed bug in sqlite reflections were INTEGER type does not take args #73

Closed
wants to merge 1 commit into from

Conversation

fergalwalsh
Copy link

INTEGER type no longer takes arguments so a TypeError: object() takes no parameters exception was being raised when doing reflection with a sqlite column type of int(5) for example.

This fix ensures no args are passed when creating a INTEGER type.

@zzzeek
Copy link
Owner

zzzeek commented Feb 16, 2014

OK the problem with this is that it's too specific. There's lots of numeric types like BIGINT SMALLINT and all that which also don't accept arguments, and all kinds of other types that dont accept arguments either. SQLite is this special-case database where its typing system is based on essentially a regular expression, so in theory the way we do things here should figure out any and all potential argument conflicts and ignore them all. In this regard there is also a pullreq that will be merged #65 which addresses the issue of type names that are not known at all. what i'd like to do with this issue is either catch TypeError (less sophisticated) or better have a lookup mapping of base types (like String, Integer, etc.) to some structure that refers to what arguments we may want to accept. Really, we could just be ignoring all the args in all cases since they don't matter anyway, though it is nice to reflect things like lengths for strings, precisions and scales for numerics, etc.

If you need a quick fix for this, I'll accept a pullreq that a. catches TypeError and b. emits a warning for that TypeError, then ignores the args. it also needs to have some tests. thanks !

@zzzeek zzzeek closed this Feb 16, 2014
@zzzeek
Copy link
Owner

zzzeek commented Feb 16, 2014

I've implemented that fix in 5e100a7

@fergalwalsh
Copy link
Author

Thanks for the explanation and the quick fix. That's great.

or better have a lookup mapping of base types (like String, Integer, etc.) to some structure that refers to what arguments we may want to accept.

Perhaps these argument lists could be an attribute of the base types themselves so that you could check len(coltype.arg_list) == len(args) or something similar? I was going to suggest using inspect.getargspec(coltype.__init__) for this but it throws an exception when you havn't overridden object.__init__ so it's a little messy but would work I think.

Anyway, thanks again for the current fix.

@zzzeek
Copy link
Owner

zzzeek commented Feb 17, 2014

yeah a getargspec() approach would probably be needed here, we already do similar things with for example generic_repr(), but I think the TypeError thing is good enough for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants