Skip to content

don't assume property is not nullable when no "nullable" field on property #55

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

Merged
merged 1 commit into from
Jul 26, 2017

Conversation

nbushak
Copy link
Contributor

@nbushak nbushak commented Jun 21, 2017

Most of the time, the properties set on SQLAlchemy models are the Column datatype. These are guaranteed to have a nullable attribute set of either True or False:
https://github.com/zzzeek/sqlalchemy/blob/master/lib/sqlalchemy/sql/schema.py#L1210

I tested this by print()ing the value of .nullable for all my properties in is_column_nullable(). All were set, and always True or False.

However, column_property() properties do not have a nullable attribute set. It's incorrect to assume these properties are not nullable. Here is an example, which returns null and throws in graphene when func.count(PersonInfo.id) is 0:

Reachout.replied_percent = column_property(
    select([
        func.sum(func.cast(PersonInfo.has_replied, Integer)) /\
            func.cast(func.count(PersonInfo.id), Float)
    ]).where(
        PersonInfo.reachout_id == Reachout.id,
    ),
)

@coveralls
Copy link

coveralls commented Jun 21, 2017

Coverage Status

Coverage remained the same at 91.129% when pulling 2c5b103 on nbushak:master into c1a0cae on graphql-python:master.

@syrusakbary syrusakbary merged commit 3aa06b6 into graphql-python:master Jul 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants