-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Messenger] fix oracle sequence insert #54176
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
[Messenger] fix oracle sequence insert #54176
Conversation
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. 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.
Can you spot the PR that introduced the regression for you?
It's better when we don't have platform-specific code paths so it'd be nice to be 100% sure we do need this special check.
@@ -127,32 +127,52 @@ public static function buildConfiguration(#[\SensitiveParameter] string $dsn, ar | |||
*/ | |||
public function send(string $body, array $headers, int $delay = 0): string | |||
{ | |||
|
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.
to be removed
$now = new \DateTimeImmutable('UTC'); | ||
$availableAt = $now->modify(sprintf('%+d seconds', $delay / 1000)); | ||
$availableAt = $now->modify(sprintf('+%d seconds', $delay / 1000)); |
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.
why did you change this line?
it was updated quite recently, see #53187
@stefan-kamsker Do you have time to have a look at the comments? |
Closing as there is no more activities. Feel free to reopen when you have time to address the comments. |
we have a issue with oracle platform when updating to doctrine-messenger 6.x
PHP 8.1
Oracle Database 19c Enterprise Edition Release 19.0.0.0.0
PHP OCI 3.2.1 extension for PHP 8.1
Oracle Instaclient Driver 19.22
MESSENGER_TRANSPORT_DSN=doctrine://default?table_name=messenger_messages&auto_setup=false
the sequence fetch was not done anymore for messenger_messages table like in previous versions
INSERT INTO messenger_messages (BODY...) VALUES('body' ..)
this PR tries to fetch the sequence again
SELECT messenger_messages_seq.nextval FROM DUAL
INSERT INTO messenger_messages (ID,BODY...) VALUES(1,'body' ..)
we tried to fix send method with this PR, maybe there is a more elegant way to do this or any advice
thank you