sqlmodel icon indicating copy to clipboard operation
sqlmodel copied to clipboard

Proper handling of None in WHERE condition

Open mmlynarik opened this issue 3 years ago • 13 comments

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

mmlynarik avatar Sep 22 '21 12:09 mmlynarik

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.

yasamoka avatar Sep 25 '21 12:09 yasamoka

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

s-c-p avatar Oct 26 '21 17:10 s-c-p

@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:)

mmlynarik avatar Oct 28 '21 19:10 mmlynarik

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

s-c-p avatar Nov 04 '21 02:11 s-c-p

You can use Hero.age.is_(None)

tc-imba avatar Nov 16 '21 18:11 tc-imba

@tc-imba where to import is_?

northtree avatar Feb 20 '22 12:02 northtree

@tc-imba where to import is_?

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

yasamoka avatar Feb 20 '22 12:02 yasamoka

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.

NMertsch avatar Mar 17 '22 20:03 NMertsch

For IS NOT NULL, you could use is_not

where(Hero.age.is_not(None))

northtree avatar Mar 18 '22 11:03 northtree

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 avatar May 06 '22 09:05 Cielquan

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

Cielquan avatar May 06 '22 09:05 Cielquan