Cannot update one table with a subset of fields, but another table works

[waldner]

Sorry for the silly title, but here goes a minimal reproducer (this is distilled from a larger API codebase, so don't mind the style and type conversions):

Piccolo table classes:

from piccolo.columns import Text, Timestamp, Date, Serial, ForeignKey
from piccolo.table import Table

class PiccoloPieces(Table, tablename="pieces"):
    id = Serial(primary_key=True, null=False)
    type = Text(null=False, required=True)
    description = Text(null=False, required=True)
    start_date = Date(null=False, required=True)
    timezone = Text(null=False, required=True)
    internal_id = Text(null=False, required=True)

class PiccoloParts(Table, tablename="parts"):
    id = Serial(primary_key=True, null=False)
    piece_id = ForeignKey(references=PiccoloPieces, target_column='id', null=False, required=True)
    description = Text(null=False, required=True)
    start_date = Date(null=False, required=True)
    internal_id = Text(null=False, required=True)

The "pieces" table has an additional unique index on (type, internal_id); the "parts" table has an additional unique index on (piece_id, internal_id).

Code:

import asyncio
import datetime
from piccolo.engine import engine_finder
from pydantic import BaseModel, ConfigDict
from xxx.tables import PiccoloPieces, PiccoloParts

# pydantic models
class Piece(BaseModel):
    id: int
    type: str
    description: str
    start_date: datetime.date
    timezone: str
    internal_id: str

    model_config = ConfigDict(extra='forbid')

class PieceCreate(BaseModel):
    type: str
    description: str
    start_date: datetime.date
    timezone: str
    internal_id: str

    model_config = ConfigDict(extra='forbid')

class PieceUpdate(BaseModel):
    description: str
    start_date: datetime.date
    timezone: str

    model_config = ConfigDict(extra='forbid')

class Part(BaseModel):
    id: int
    piece_id: int            # foreign key
    description: str
    start_date: datetime.date
    internal_id: str

    model_config = ConfigDict(extra='forbid')

# derived pydantic types
class PartCreate(BaseModel):
    piece_id: int
    description: str
    start_date: datetime.date
    internal_id: str

    model_config = ConfigDict(extra='forbid')


class PartUpdate(BaseModel):
    description: str
    start_date: datetime.date

    model_config = ConfigDict(extra='forbid')



async def main():

    engine = engine_finder()
    await engine.start_connection_pool()

    # create a piece
    piece = PieceCreate(
      type="piecetype",
      description="piece1",
      start_date=datetime.date(2023, 1, 1),
      timezone="Europe/Madrid",
      internal_id="piece1internal",
    )
    piece_dict = piece.model_dump()

    piece_orm = await PiccoloPieces(**piece_dict).save().returning(*PiccoloPieces.all_columns())
    # [{'id': 1, 'type': 'piecetype', 'description': 'piece1', 'start_date': datetime.date(2023, 1, 1), 'timezone': 'Europe/Madrid', 'internal_id': 'piece1internal'}]

    piece_id = piece_orm[0]['id']

    # update the piece; this uses PieceUpdate, which has a subset of fields, and
    # works fine
    piece = PieceUpdate(
      description="piece1 new description",
      start_date=datetime.date(2023, 2, 1),
      timezone="Europe/Rome",
    )

    piece_dict = piece.model_dump()
    updated_piece_orm = await PiccoloPieces().update(**piece_dict).where(
            (PiccoloPieces.id == piece_id) ).returning(
            *PiccoloPieces.all_columns())

    # create a part
    part = PartCreate(
        piece_id=piece_id,
        description="part1",
        start_date=datetime.date(2024, 1, 1),
        internal_id="part1internal",
    )
    part_dict = part.model_dump()

    part_orm = await PiccoloParts(**part_dict).save().returning(*PiccoloParts.all_columns())
    #[{'id': 1, 'piece_id': 1, 'description': 'part1', 'start_date': datetime.date(2024, 1, 1), 'internal_id': 'part1internal'}]

    part_id = part_orm[0]['id']

    # update the part; this uses a subset of fields, and produces an exception "piece_id was not provided"
    part = PartUpdate(
      description="part1 new description",
      start_date=datetime.date(2023, 3, 1),
    )

    part_dict = part.model_dump()
    updated_part_orm = await PiccoloParts().update(**part_dict).where(
            (PiccoloParts.id == part_id) ).returning(
            *PiccoloParts.all_columns())    # ValueError: piece_id wasn't provided

    print(updated_part_orm)

asyncio.run(main())

So the update to the part produces an exception, whereas the update to the piece works, in both cases I'm using a subset of fields. I'm not sure I'm missing something here; my goal is that the only updatable columns for parts after creation should be description and start_date, so I should be able to receive a PartUpdate and write it to the db, updating only those columns (the same way I do with PieceUpdate, which works).

[sinisaos]

@waldner I think the main problem is that you set null=False on the ForeignKey column and it looks similar to this issue. If you set null=True (which is the default), everything works.

[waldner]

Thanks, I will test ASAP. However, using null=True on a foreign key field seems a bit odd...I truly do want the field to not be null.

[sinisaos]

I don't know if this is a good solution, but we can change this https://github.com/piccolo-orm/piccolo/blob/f9c77cc89dd3f5ef6d64ac6426395b684e70bbbe/piccolo/table.py#L435--L440 to omit ForeignKey columns from checks like this

if (
    (value is None)
    and (not column._meta.null)
    and not _ignore_missing
    and not isinstance(column, ForeignKey) # new line
):
    raise ValueError(f"{column._meta.name} wasn't provided")

This way we can declare null True or False without raising ValueError. Here is branch

[waldner]

Yes, setting null=True on the foreign key column works. Still, as I said, it looks counterintuituve.