PHP8 attributes do not respect uniqueConstraint property from the Table attribute #6935

Open
opened 2026-01-22 15:41:43 +01:00 by admin · 7 comments
Owner

Originally created by @pkly on GitHub (Feb 22, 2022).

Bug Report

Q A
BC Break yes
Version 2.11.1 (and possibly earlier)

Summary

After converting from annotations to attributes I wanted to keep my configuration the same, but it appears that the uniqueConstraints property is not being read.
By directly placing a UniqueConstraint attribute on the entity the behavior is as expected.

Current behavior

Migrations create invalid changes by removing unique indexes from tables. UniqueConstraints are not being recognized.

How to reproduce

With a minimal entity use the following mappings:

// PHP8.1 (working)
#[ORM\Entity(repositoryClass: MyRepository:class)]
#[ORM\Table(name: 'my_table')]
#[ORM\UniqueConstraint(name: 'my_unique_index', columns: ['some_id',])]
// PHP8.1 (not working)
#[ORM\Entity(repositoryClass: MyRepository:class)]
#[ORM\Table(name: 'my_table', uniqueConstraints: [
    new ORM\UniqueConstraint(name: 'my_unique_index', columns: ['some_id',])
])]
// PHP7.4-ish (annotations) (working)
/**
 * @ORM\Entity(repositoryClass=MyRepository::class)
 * @ORM\Table(name="my_table", uniqueConstraints={
 *     @ORM\UniqueConstraint(name="my_unique_index", columns={"some_id"})
 * })
 */

Expected behavior

For the array of attributes to work with UniqueConstraint in the table attribute as the uniqueConstraints parameter.

It was not listed in the upgrade docs, so I'm assuming it's a BC break.

Originally created by @pkly on GitHub (Feb 22, 2022). ### Bug Report | Q | A |------------ | ------ | BC Break | yes | Version | 2.11.1 (and possibly earlier) #### Summary After converting from annotations to attributes I wanted to keep my configuration the same, but it appears that the `uniqueConstraints` property is not being read. By directly placing a UniqueConstraint attribute on the entity the behavior is as expected. #### Current behavior Migrations create invalid changes by removing unique indexes from tables. UniqueConstraints are not being recognized. #### How to reproduce With a minimal entity use the following mappings: ```php // PHP8.1 (working) #[ORM\Entity(repositoryClass: MyRepository:class)] #[ORM\Table(name: 'my_table')] #[ORM\UniqueConstraint(name: 'my_unique_index', columns: ['some_id',])] ``` ```php // PHP8.1 (not working) #[ORM\Entity(repositoryClass: MyRepository:class)] #[ORM\Table(name: 'my_table', uniqueConstraints: [ new ORM\UniqueConstraint(name: 'my_unique_index', columns: ['some_id',]) ])] ``` ```php // PHP7.4-ish (annotations) (working) /** * @ORM\Entity(repositoryClass=MyRepository::class) * @ORM\Table(name="my_table", uniqueConstraints={ * @ORM\UniqueConstraint(name="my_unique_index", columns={"some_id"}) * }) */ ``` #### Expected behavior For the array of attributes to work with UniqueConstraint in the table attribute as the uniqueConstraints parameter. It was not listed in the upgrade docs, so I'm assuming it's a BC break.
Author
Owner

@derrabus commented on GitHub (Feb 22, 2022):

Your first example is the correct and documented way to declare a unique constraint with attributes.

It was not listed in the upgrade docs, so I'm assuming it's a BC break.

Since the code in your second example has never worked, it's by definition not a BC break.

Some background on this: When we introduced attributes, nesting them was not possible yet. I understand that having that constructor parameter causes confusion and I agree that we should do somthing about it.

Since we cannot remove the parameter because we need it for Doctrine Annotations, maybe we should allow both ways (first and second example) for now and remove one of the ways in 3.0?

/cc @beberlei

@derrabus commented on GitHub (Feb 22, 2022): Your first example is the correct and documented way to declare a unique constraint with attributes. > It was not listed in the upgrade docs, so I'm assuming it's a BC break. Since the code in your second example has never worked, it's by definition not a BC break. Some background on this: When we introduced attributes, nesting them was not possible yet. I understand that having that constructor parameter causes confusion and I agree that we should do somthing about it. Since we cannot remove the parameter because we need it for Doctrine Annotations, maybe we should allow both ways (first and second example) for now and remove one of the ways in 3.0? /cc @beberlei
Author
Owner

@pkly commented on GitHub (Feb 22, 2022):

Since an in-place upgrade is not possible I'd still consider it a BC break...

I understand why it doesn't work, but seeing how the 8.1 new in initializers were accepted a while back (https://wiki.php.net/rfc/new_in_initializers) this likely could've been avoided.
As attributes are not yet documented on the doctrine docs for ORM at all I don't think it's not a bc break, since if I didn't find the reader looking at class annotations by myself it'd be rather painful.
Since the argument is there and is needed for annotations I'd highly suggest you still allow it to be used in this use-case. More so because nothing on the attribute even mentions that this element is deprecated or a different usage is suggested.
Maybe if attributes are used and this is detected you should simply add it and fire off a deprecation notice for 3.x?

@pkly commented on GitHub (Feb 22, 2022): Since an in-place upgrade is not possible I'd still consider it a BC break... I understand why it doesn't work, but seeing how the 8.1 new in initializers were accepted a while back (https://wiki.php.net/rfc/new_in_initializers) this likely could've been avoided. As attributes are not yet documented on the doctrine docs for ORM _at all_ I don't think it's _not_ a bc break, since if I didn't find the reader looking at class annotations by myself it'd be rather painful. Since the argument is there and is needed for annotations I'd highly suggest you still allow it to be used in this use-case. More so because nothing on the attribute even mentions that this element is deprecated or a different usage is suggested. Maybe if attributes are used and this is detected you should simply add it and fire off a deprecation notice for 3.x?
Author
Owner

@beberlei commented on GitHub (Feb 22, 2022):

@derrabus we discussed allowing both ways in #9240 and decided against because of the complexity and confusion of two ways. I'd rather instead throw an exception if Table#uniqueConstraints isset, pointing to the right way. Same for other nested ways that we flattened (Table indexes and JoinTable join columns for example).

@beberlei commented on GitHub (Feb 22, 2022): @derrabus we discussed allowing both ways in #9240 and decided against because of the complexity and confusion of two ways. I'd rather instead throw an exception if Table#uniqueConstraints isset, pointing to the right way. Same for other nested ways that we flattened (Table indexes and JoinTable join columns for example).
Author
Owner

@beberlei commented on GitHub (Feb 22, 2022):

@pkly not sure what you mean, attribute mappings are completly documneted here https://www.doctrine-project.org/projects/doctrine-orm/en/2.11/reference/attributes-reference.html#attributes-reference

@beberlei commented on GitHub (Feb 22, 2022): @pkly not sure what you mean, attribute mappings are completly documneted here https://www.doctrine-project.org/projects/doctrine-orm/en/2.11/reference/attributes-reference.html#attributes-reference
Author
Owner

@pkly commented on GitHub (Feb 22, 2022):

@beberlei sorry, I meant the "Mapping Objects onto a Database" section for 2.11.
Attributes are not listed there, I think I found the attribute reference once via google directly.

As for the solution - throwing sounds like a good idea too, it's just that silently failing on something what is essentially a configuration for a program is not a great idea, and rather confusing.

On top of that the examples in other pages still don't list attributes at all, and since those are not compatible 1:1....

@pkly commented on GitHub (Feb 22, 2022): @beberlei sorry, I meant the "Mapping Objects onto a Database" section for 2.11. Attributes are not listed there, I think I found the attribute reference once via google directly. As for the solution - throwing sounds like a good idea too, it's just that silently failing on something what is essentially a configuration for a program is not a great idea, and rather confusing. On top of that the examples in other pages still don't list attributes at all, and since those are not compatible 1:1....
Author
Owner

@derrabus commented on GitHub (Feb 22, 2022):

Works for me as well!

@derrabus commented on GitHub (Feb 22, 2022): Works for me as well!
Author
Owner

@BonBonSlick commented on GitHub (Feb 13, 2024):

  • 1 - parentheses for option
  • 2 - parentheses for each condition if you have AND
  • 3 - lowercase "where"
  • 4 - lower case "true / false" - this and all above may not generate sql at all
  • 5 - do not use NOT, instead use "field = false", with NOT it will generate each time new sql
@BonBonSlick commented on GitHub (Feb 13, 2024): - 1 - parentheses for option - 2 - parentheses for each condition if you have AND - 3 - lowercase "where" - 4 - lower case "true / false" - this and all above may not generate sql at all - 5 - do not use NOT, instead use "field = false", with NOT it will generate each time new sql
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#6935