piccolo-orm / piccolo

A fast, user friendly ORM and query builder which supports asyncio.
https://piccolo-orm.com/
MIT License
1.45k stars 91 forks source link

`TimestampNow` generates inconsistant values. #919

Open HakierGrzonzo opened 10 months ago

HakierGrzonzo commented 10 months ago

TimestampNow generates values for default Timestamp columns in the ORM code, not in the DB.

As a result, one row with many columns that use TimestampNow as a default value will have inconsistent data.

See this example code:

from piccolo.columns.defaults.timestamp import TimestampNow
from piccolo.engine import SQLiteEngine
from piccolo.table import Table
from piccolo.columns import Timestamp

import asyncio

DB = SQLiteEngine(path="./some_db.sqlite")

class Example(Table, db=DB):
    first = Timestamp(default=TimestampNow())
    second = Timestamp(default=TimestampNow())

async def main():
    await Example.create_table()
    await Example.insert(
        Example()
    )

    data = await Example.select()
    row = data[0]
    print(row)
    assert row[Example.first] == row[Example.second]

asyncio.run(main())

It will fail with the following error:

  File "wherever/test.py", line 23, in main
    assert row[Example.first] == row[Example.second]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

As the values in the row are:

{
  'id': 1, 
  'first': datetime.datetime(2024, 1, 15, 12, 42, 58, 66463), 
  'second': datetime.datetime(2024, 1, 15, 12, 42, 58, 66467)
}

Any directions how could I fix this bug?

dantownsend commented 10 months ago

@HakierGrzonzo Yeah, that's an interesting one. I think your best solution is this:

now = datetime.datetime.now(tz=datetime.timezone.utc)

await Example.insert(
    Example(first=now, second=now)
)

We might be able to add a feature to Piccolo where the timestamps only come from the database.

dantownsend commented 10 months ago

If you want the timestamp to be generated by the database instead, this works:

from piccolo.querystring import QueryString

await Example.insert(
    Example(first=QueryString('CURRENT_TIMESTAMP'), second=QueryString('CURRENT_TIMESTAMP')),
)
HakierGrzonzo commented 10 months ago

@dantownsend

I expanded this solution and came up with this:

from piccolo.columns.defaults.timestamp import TimestampNow
from piccolo.engine import SQLiteEngine
from piccolo.querystring import QueryString
from piccolo.table import Table
from piccolo.columns import Timestamp

import asyncio

DB = SQLiteEngine(path="./some_db.sqlite")

class TimestampNow2(TimestampNow):
    def python(self):
        return QueryString("CURRENT_TIMESTAMP")

class TimestampWithFunctionDefault(Timestamp):
    """
    This is a hack to modify piccolo default parameter type checking.
    """

    def __init__(self, default: TimestampNow2, **kwargs) -> None:
        self._validate_default(default, [TimestampNow2])  # type: ignore

        super().__init__(default=None, **kwargs)
        self.default = default

    @property
    def column_type(self):
        return "TIMESTAMP"

class Example(Table, db=DB):
    first = TimestampWithFunctionDefault(default=TimestampNow2())
    second = TimestampWithFunctionDefault(default=TimestampNow2())

async def main():
    await Example.create_table()
    await Example.insert(Example())

    data = await Example.select()
    row = data[0]
    print(data)
    assert row[Example.first] == row[Example.second]

asyncio.run(main())

Now it passes the assert statement

dantownsend commented 10 months ago

@HakierGrzonzo That's a nice solution - thanks for sharing!