Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Doctrine][Messenger] Oracle sequences are suffixed with_seq#58557
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
carsonbot commentedOct 14, 2024
Hey! Thanks for your PR. You are targeting branch "6.4" but it seems your PR description refers to branch "6.4, and 7.1 for bug fixes". Cheers! Carsonbot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could this be covered by tests to avoid regressions?
| // We need to create a sequence for Oracle and set the id column to get the correct nextval | ||
| if ($this->driverConnection->getDatabasePlatform()instanceof OraclePlatform) { | ||
| $idColumn->setDefault('seq_'.$this->configuration['table_name'].'.nextval'); | ||
| $idColumn->setDefault($this->configuration['table_name'].'_seq'.'.nextval'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
| $idColumn->setDefault($this->configuration['table_name'].'_seq'.'.nextval'); | |
| $idColumn->setDefault($this->configuration['table_name'].'_seq.nextval'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@alexandre-daubois : I updated that portion of code for consistency.
@rjd22 : Is this if block really needed as messenger in it's auto config process already create a sequence and trigger in case of id not specified ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, for Oracle installations the default block is needed. Else the field won't use the sequence on some installations.
_seq882e17a to18eeb64Comparerjd22 commentedOct 17, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@xabbuh Since you looked at mine can you take a look at this one? It looks good to me. If we merge this before the next release we avoid a breaking change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
works for me
_seq_seq18eeb64 to6a17c04CompareThank you@devloop42. |
5bc387b intosymfony:6.4Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Generated sequences, by doctrine or in this case by the auto_setup of messenger, are suffixed with
_seq.