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

Proper handling of None in WHERE condition #109

Open
8 tasks done
mmlynarik opened this issue Sep 22, 2021 · 15 comments
Open
8 tasks done

Proper handling of None in WHERE condition #109

mmlynarik opened this issue Sep 22, 2021 · 15 comments
Labels
feature New feature or request

Comments

@mmlynarik
Copy link

mmlynarik commented Sep 22, 2021

First Check

  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the SQLModel documentation, with the integrated search.
  • I already searched in Google "How to X in SQLModel" and didn't find any information.
  • I already read and followed all the tutorial in the docs and didn't find an answer.
  • I already checked if it is not related to SQLModel but to Pydantic.
  • I already checked if it is not related to SQLModel but to SQLAlchemy.

Commit to Help

  • I commit to help with one of those options 👆

Example Code

from typing import Optional

from sqlmodel import Field, Session, SQLModel, create_engine, select, col


class Hero(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
    name: str
    secret_name: str
    age: Optional[int] = None


sqlite_file_name = "database.db"
sqlite_url = f"sqlite:///{sqlite_file_name}"

engine = create_engine(sqlite_url, echo=True)


def create_db_and_tables():
    SQLModel.metadata.create_all(engine)


def create_heroes():
    hero_1 = Hero(name="Deadpond", secret_name="Dive Wilson")
    hero_2 = Hero(name="Spider-Boy", secret_name="Pedro Parqueador")
    hero_3 = Hero(name="Rusty-Man", secret_name="Tommy Sharp", age=48)

    with Session(engine) as session:
        session.add_all([hero_1, hero_2, hero_3])
        session.commit()


def select_heroes():
    with Session(engine) as session:
        statement = select(Hero).where(col(Hero.age) is None, Hero.name.like('%Deadpond%'))
        results = session.exec(statement)
        for hero in results:
            print(hero)

Description

To make the where condition Hero.age is None functional, because currently it is evaluated as false even though it should be true. Otherwise, one needs to use Hero.age == None, which raises linting error.

Wanted Solution

To make the where condition Hero.age is None functional, because currently it is evaluated as false even though it should be true. Otherwise, one needs to use Hero.age == None, which raises linting error.

Wanted Code

from typing import Optional

from sqlmodel import Field, Session, SQLModel, create_engine, select, col


class Hero(SQLModel, table=True):
    id: Optional[int] = Field(default=None, primary_key=True)
    name: str
    secret_name: str
    age: Optional[int] = None


sqlite_file_name = "database.db"
sqlite_url = f"sqlite:///{sqlite_file_name}"

engine = create_engine(sqlite_url, echo=True)


def create_db_and_tables():
    SQLModel.metadata.create_all(engine)


def create_heroes():
    hero_1 = Hero(name="Deadpond", secret_name="Dive Wilson")
    hero_2 = Hero(name="Spider-Boy", secret_name="Pedro Parqueador")
    hero_3 = Hero(name="Rusty-Man", secret_name="Tommy Sharp", age=48)

    with Session(engine) as session:
        session.add_all([hero_1, hero_2, hero_3])
        session.commit()


def select_heroes():
    with Session(engine) as session:
        statement = select(Hero).where(col(Hero.age) is None, Hero.name.like('%Deadpond%'))
        results = session.exec(statement)
        for hero in results:
            print(hero)

Alternatives

No response

Operating System

Linux

Operating System Details

No response

SQLModel Version

0.0.4

Python Version

3.8.5

Additional Context

No response

@mmlynarik mmlynarik added the feature New feature or request label Sep 22, 2021
@yasamoka
Copy link

yasamoka commented Sep 25, 2021

I believe this is concerned entirely with SQLAlchemy, not with SQLModel, and has to do with the required semantics to construct a BinaryExpression object.

Hero.age == None evaluates to a BinaryExpression object which is eventually used to construct the SQL query that the SQLAlchemy engine issues to your DBMS.

Hero.age is None evaluates to False immediately, and not a BinaryExpression, which short-circuits the query no matter the value of age in a row.

From a cursory search, it does not seem that the is operator can be overridden in Python. This could help explain why the only possibility is by using the == operator, which can be overridden.

@s-c-p
Copy link

s-c-p commented Oct 26, 2021

@mmlynarik Along the lines of .like(), would it be acceptable if we implement something like Hero.age.is(None)?

@mmlynarik
Copy link
Author

@mmlynarik Along the lines of .like(), would it be acceptable if we implement something like Hero.age.is(None)?

If it worked as expected, then definitely yes:)

@s-c-p
Copy link

s-c-p commented Nov 4, 2021

Ok, I am working on a PR implementing this, repo owners please give me some time 👍

@tc-imba
Copy link

tc-imba commented Nov 16, 2021

You can use Hero.age.is_(None)

@northtree
Copy link

@tc-imba where to import is_?

@yasamoka
Copy link

yasamoka commented Feb 20, 2022

@tc-imba where to import is_?

is_ is an attribute. You don't need to import it to use it.

@NMertsch
Copy link

My understanding is this:

where(Hero.age is None) cannot be implemented, because the is operator cannot be overloaded (== can be overloaded via the __eq__ method, etc.).

where(Hero.age.is(None)) cannot be implemented, because is is a reserved keyword and therefore no valid identifier.

where(Hero.age.is_(None)) works and probably is the closest we can get. It follows PEP 8 Naming Styles by using a trailing underscore to "avoid conflicts with Python keyword".

But is_ is not working with autocompletion, at least not in Pycharm. Hero.age is inferred to be int | None, which is exactly what I want in 99% of cases. But it means that Hero.age.is_ raises a warning "unresolved reference".
Not sure there is a good way to deal with it.

@northtree
Copy link

For IS NOT NULL, you could use is_not

where(Hero.age.is_not(None))

@Cielquan
Copy link

Cielquan commented May 6, 2022

My understanding is this:

where(Hero.age is None) cannot be implemented, because the is operator cannot be overloaded (== can be overloaded via the __eq__ method, etc.).

where(Hero.age.is(None)) cannot be implemented, because is is a reserved keyword and therefore no valid identifier.

where(Hero.age.is_(None)) works and probably is the closest we can get. It follows PEP 8 Naming Styles by using a trailing underscore to "avoid conflicts with Python keyword".

But is_ is not working with autocompletion, at least not in Pycharm. Hero.age is inferred to be int | None, which is exactly what I want in 99% of cases. But it means that Hero.age.is_ raises a warning "unresolved reference". Not sure there is a good way to deal with it.

The same goes for the in_ operator method (an presumably all the other operators as well):

select(Ticket).where(Ticket.stage_id.in_(stage_ids))

This also let mypy and other type using tool cry out loud like "int" has no attribute "in_" [attr-defined]mypy(error)

@Cielquan
Copy link

Cielquan commented May 6, 2022

I guess the issue title could be updated to smth like Proper handling of column operator methods in WHERE condition

@patrickwasp
Copy link

looking for a team without heros like

select(Team).where(Team.heros == None)

isn't compatible with E711

but select(Team).where(Team.heros is None)

doesn't work

@NMertsch
Copy link

NMertsch commented Jun 7, 2024

Team.heros is None cannot be made to work because the Python language does not allow overloading is, but does allow overloading ==.

Team.heros == None checks for equality, not identity. It seems to do what you want, but it is a PEP 8 violation.

Team.heros.is_(None) should also do what you want, and is more idiomatic because it checks for identity (like is), not equality (like ==), following the PEP 8 recommendation.

In the end, write the code that works for you. PEP 8 is just a recommendation, and flake8 is just a tool. If you prefer to ignore a recommendation, PEP 8 has a section about that as well.

@patrickwasp
Copy link

Besides using # type: ignore is there an easy way to make pylance happy? Attribute "is_" is unknown Pylance

@gerrior
Copy link

gerrior commented Nov 9, 2024

@patrickwasp this issue is still open. It hasn't be merged to the code base yet.

I recommend the following code:

select(Team).where(
            Team.heros == None,  # noqa E711 Comparison to `None` should be `cond is None`.
)
select(Team).where(
            Team.heros is None,  # This does not work as intended.
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants