NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

Embedded identifier support#16826

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

mihai-stancu wants to merge1 commit intosymfony:2.8frommihai-stancu:embedded_identifier_support

Closed

Embedded identifier support#16826

mihai-stancu wants to merge1 commit intosymfony:2.8frommihai-stancu:embedded_identifier_support

Conversation

Copy link

Contributor

mihai-stancu commentedDec 3, 2015

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	none
License	MIT
Doc PR	n/a

If you define your entity identifier using an embeddable class the identifier name will contain period symbols in it such asid.value which will generate a syntax error in the DQL lexer because parameter names are not allowed to contain period symbols.

Example to reproduce described bug:

/** * @ORM\Embeddable */class Identifier {/**     * @ORM\Column(type="integer")     */protected$value;}/** * @ORM\Entity */class Entity {/**     * @ORM\Id @ORM\Embedded(class="Identifier")     */protected$id;}

carsonbot added the Status: Needs Review label

Dec 3, 2015

Copy link

Contributor

sstok commentedDec 8, 2015

Can you add a test to prevent future regressions?

Status: Needs work

carsonbot added Status: Needs Work and removed Status: Needs Review labels

Dec 8, 2015

Copy link

ContributorAuthor

mihai-stancu commentedDec 8, 2015

Sure, will come back with the test for this soon.

I just want to make sure there's no counter opinion relating to this (such as "why would you use embedded objects as entity identifiers").

Embedded identifier support

3d83547

Copy link

ContributorAuthor

mihai-stancu commentedDec 22, 2015

@sstok sorry for the delay, I added the suggested tests.

jakzal added Status: Needs Review and removed Status: Needs Work labels

Dec 23, 2015

Copy link

Member

fabpot commentedDec 26, 2015

Thank you@mihai-stancu.

fabpot added a commit that referenced this pull request

Dec 26, 2015

bug#16826Embedded identifier support (mihai-stancu)

021ab8a

This PR was submitted for the 2.8 branch but it was merged into the 2.3 branch instead (closes#16826).Discussion----------Embedded identifier support| Q             | A| ------------- | ---| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | none| License       | MIT| Doc PR        | n/aIf you define your entity identifier using an embeddable class the identifier name will contain period symbols in it such as `id.value` which will generate a syntax error in the DQL lexer because parameter names are not allowed to contain period symbols.Example to reproduce described bug:```php/** *@Orm\Embeddable */class Identifier {    /**     *@Orm\Column(type="integer")     */    protected $value;}/** *@Orm\Entity */class Entity {    /**     *@Orm\Id@Orm\Embedded(class="Identifier")     */    protected $id;}```Commits-------38fdda6 Embedded identifier support