get_or_create raises ValueError

[bymoye]

python_version: 3.13.1
piccolo-orm version: 1.22.0

I created the following table：

class GroupMember(Table):
    id = UUIDv7(primary_key=True, required=True)
    group_id = columns.ForeignKey(references=Group, index=True, null=False)
    account_id = columns.ForeignKey(references=Account, index=True, null=False)

Then：

group_id = "01950e3a-df45-7691-a9d8-52ee9ef1ea66"
user_id = "01950e3a-df40-7392-bc47-5a53cda0f24f"
# The following code will throw an exception: ValueError: group_id wasn't provided
await GroupMember.objects().get_or_create((GroupMember.group_id == group_id) & (GroupMember.account_id == user_id))
# But the following code does not throw an exception.
if not (member := await GroupMember.objects().get(
    (GroupMember.group_id == group_id) & (GroupMember.account_id == user_id)
)):
    await GroupMember.objects().create(
        group_id = group_id,
        account_id = user_id,
    )

# Neither will the following code.
if not (member := await GroupMember.objects().get(
    (GroupMember.group_id == group_id) & (GroupMember.account_id == user_id)
)):
    await GroupMember({GroupMember.group_id: group_id, GroupMember.account_id: user_id}).save()

I don't know why.

[dantownsend]

Hmm, I'm not sure - your code looks OK to me.

I created this reproducible example:

import asyncio

from piccolo.table import Table, create_db_tables
from piccolo.columns import ForeignKey, Varchar, Serial
from piccolo.engine.postgres import PostgresEngine


DB = PostgresEngine({
    "database": "piccolo_get_or_create"
})


class Group(Table, db=DB):
    id: Serial
    name = Varchar()


class Account(Table, db=DB):
    id: Serial
    name = Varchar()


class GroupMember(Table, db=DB):
    id: Serial
    group_id = ForeignKey(references=Group, index=True, null=False)
    account_id = ForeignKey(references=Account, index=True, null=False)


async def main():
    await create_db_tables(Group, Account, GroupMember, if_not_exists=True)

    group = Group()
    await group.save()

    account = Account()
    await account.save()

    await GroupMember.objects().get_or_create(
        (
            GroupMember.group_id == group.id
        ) & (
            GroupMember.account_id == account.id
        )
    )


if __name__ == '__main__':
    asyncio.run(main())

It could be a bug - I would use one of the other syntaxes for now while we investigate.

[sinisaos]

@dantownsend The get_or_create method seems to be different for FK columns. First I set null to True in the group_id and account_id columns (which is the default null value in FK columns as opposed to normal columns where null defaults to False), and after that everything works on the Serial and UUID primary keys. But it's not a good solution and I think the problem lies in this part of the code

piccolo/piccolo/table.py

Lines 435 to 440 in 8fd69e7

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

The default value for FK columns is always None, but when we set null=False for group_id and account_id columns, a ValueError occurs, otherwise not (I'm not exactly clear why). Should we set the value as Ellipsis as default if None is provided like this value is ...? After that change, everything works regardless of the null value. I hope that somehow helps in understanding this problem.

[dantownsend]

@sinisaos Interesting - so it's something to do with having null=False. I just tried it with null=True and it works.

It looks like this is the problem:

piccolo/piccolo/query/methods/objects.py

Line 68 in 8fd69e7

instance = self.table_class(_data=self.defaults)

If we change it to this, then it works:

instance = self.table_class(_data=self.defaults, _ignore_missing=True)

By default, when instantiating a Table, we validate that all of the values were provided.

Rather than setting _ignore_missing=True, a better solution would be:

1. Get all of the values from the where
   

   piccolo/piccolo/query/methods/objects.py

   Lines 72 to 83 in 8fd69e7

   	if isinstance(self.where, Where):
   	setattr(
   	instance,
   	self.where.column._meta.name, # type: ignore
   	self.where.value, # type: ignore
   	)
   	elif isinstance(self.where, And):
   	for column, value in self.where.get_column_values().items():
   	if len(column._meta.call_chain) == 0:
   	# Make sure we only set the value if the column belongs
   	# to this table.
   	setattr(instance, column._meta.name, value)

2. Combine with self.defaults.
3. Then instantiate the Table.

[bymoye]

Sorry, a suggestion that is off topic:
column._meta.call_chain is a List, then
if len(column._meta.call_chain) == 0 can be changed to if not column._meta.call_chain
because the latter is more efficient than the former