-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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 comparison error when scale is None #4638
Conversation
Prevents `'>' not supported between instances of 'NoneType' and 'int'` error when scale is `None`
redash/query_runner/oracle.py
Outdated
@@ -35,7 +35,7 @@ class Oracle(BaseSQLQueryRunner): | |||
@classmethod | |||
def get_col_type(cls, col_type, scale): | |||
if col_type == cx_Oracle.NUMBER: | |||
return TYPE_FLOAT if scale > 0 else TYPE_INTEGER | |||
return TYPE_INTEGER if scale is False else TYPE_FLOAT |
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.
If scale
can be None
, then in such cases it will be classified as a float. Is this correct? If not, you might want to change the if statement.
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.
My understanding is that if scale is not None
or is > 0
(so basically it is a truthy value) it should be classified as a Float, otherwise Integer.
However, here is the definition from cx_Oracle
for NUMBER
:
If the column is unconstrained (no precision and scale specified), the value will be returned as a float or an int depending on whether the value itself is an integer. In all other cases the value is returned as a float.
Seems like None
does not always mean Integer
. Maybe we should look at the value itself whether it is an int or float to decide at this point?
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.
Maybe we should look at the value itself whether it is an int or float to decide at this point?
Sounds like the safest option, no?
What type of PR is this? (check all applicable)
Description
Prevents
'>' not supported between instances of 'NoneType' and 'int'
error when scale isNone
Related Tickets & Documents
Closes #4786.