Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Self transation cannot be resolved. #489

Open
1 task done
peldax opened this issue Jun 23, 2024 · 4 comments · May be fixed by #491
Open
1 task done

🐛 Self transation cannot be resolved. #489

peldax opened this issue Jun 23, 2024 · 4 comments · May be fixed by #491
Labels

Comments

@peldax
Copy link

peldax commented Jun 23, 2024

No duplicates 🥲.

  • I have searched for a similar issue in our bug tracker and didn't find any solutions.

What happened?

Hi,

I am getting a following error when persisting a self referencing entitiy, such as following:

#[Entity(database: 'main')]
class Catalogue
{
    #[Column(type: 'primary')]
    private int $id;

    #[RefersTo(target: self::class, nullable: true)]
    private ?self $catalogue = null;

    #[HasMany(target: self::class)]
    private array $catalogues = [];
}

Error message:

Cycle\ORM\Exception\TransactionException: Transaction can't be finished. Some relations can't be resolved:
Create new `catalogue`
 - catalogue.catalogues:catalogue (Cycle\ORM\Relation\ShadowBelongsTo)

It seems that the ORM is having trouble understanding the selfReferencing relation.

Version

- ORM 2.8
- PHP 8.3.7
@peldax peldax added status:to be verified Needs to be reproduced and validated. type:bug Bug labels Jun 23, 2024
@roxblnfk
Copy link
Member

There is test for many-to-many relation
https://github.com/cycle/orm/tree/many-to-many-self-cyclic

@roxblnfk roxblnfk removed the status:to be verified Needs to be reproduced and validated. label Jun 23, 2024
@roxblnfk roxblnfk moved this to Todo in Cycle Jun 23, 2024
@peldax
Copy link
Author

peldax commented Jun 24, 2024

Hi @roxblnfk,

I found that when the HasMany catalogues relation is marked as nullable, the persisting happens correctly.

Does it make sense that the HasMany relation must explicitly know whether the targeting entity allows null or not?
In my opinion the BelongsTo/RefersTo relations should have the final (and possibly the only) word about its nullability - this is the relevant column after all.

@roxblnfk
Copy link
Member

Hi

Does it make sense that the HasMany relation must explicitly know whether the targeting entity allows null or not? In my opinion the BelongsTo/RefersTo relations should have the final (and possibly the only) word about its nullability - this is the relevant column after all.

At the ORM level, we declare relationships, not table columns.
For example, there can be a situation: Parent#hasOne(Child, nullable=true) + Child::belongsTo(Parent, nullable=false). Here, the Parent may not have a Child because the relationship is optional in this direction. However, if a Child appears in the database, it will always have a Parent. HasOne does not affect the nullable property of the parent_id column.

However, it seems odd that the nullable property in HasMany affects anything. Could you add a test case so that we can poke around with it?

@peldax
Copy link
Author

peldax commented Jun 25, 2024

Hi,
Thanks for the reply.
I understand the usecase with HasOne, but this does not apply on the HasMany relation - there either exists some related entities or not. I think that the nullability of a target is irrelevant on the HasMany relation, please correct me if I am wrong.

Either way, it is great that it is not an intended behaviour and it should not change anything in the persisting process. I will dig deeper and try to provide a solution. I will start with the testcase.

@peldax peldax linked a pull request Jul 8, 2024 that will close this issue
@roxblnfk roxblnfk linked a pull request Jul 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

2 participants