Proper handling of None in WHERE condition

mmlynarik commented 2 years ago

First Check

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

Commit to Help

[X] 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

yasamoka commented 2 years ago

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 commented 2 years ago

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

mmlynarik commented 2 years ago

@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 commented 2 years ago

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

tc-imba commented 2 years ago

You can use Hero.age.is_(None)

northtree commented 2 years ago

@tc-imba where to import is_?

yasamoka commented 2 years ago

@tc-imba where to import is_?

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

NMertsch commented 2 years ago

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 commented 2 years ago

For IS NOT NULL, you could use is_not

where(Hero.age.is_not(None))

Cielquan commented 2 years ago

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 commented 2 years ago

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

patrickwasp commented 1 month ago

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 commented 1 month ago

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.

tiangolo / sqlmodel