Skip to content

Fix a bug that rollback a failed transaction causes server crash.#211

Merged
jkatz merged 1 commit intoaws:mainfrom
leopan3:issue208
Jul 3, 2023
Merged

Fix a bug that rollback a failed transaction causes server crash.#211
jkatz merged 1 commit intoaws:mainfrom
leopan3:issue208

Conversation

@leopan3
Copy link
Copy Markdown
Contributor

@leopan3 leopan3 commented Jul 3, 2023

Fix issue #208

The root cause of the crash is when the transaction fails, it's set to aborted state; get_extension_oid function needs a relcache lookup which must be done in a transaction state.

This change fixes this issue by skipping TransactionStmts in process utility hook.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

The root cause of the crash is when the transaction fails, it's set to aborted state; get_extension_oid function needs a relcache lookup which must be done in a transaction state.

This change fixes this issue by skipping TransactionStmts in process utility hook.
Copy link
Copy Markdown
Contributor

@jkatz jkatz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch and nice fix!

Comment on lines +3989 to +3996
if (prev_hook)
{
_prev_hook;
}
else
{
_standard_ProcessUtility;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nonblocking comment, given it's written otherwise further down on the function, but a future refactor could simplify this to:

Suggested change
if (prev_hook)
{
_prev_hook;
}
else
{
_standard_ProcessUtility;
}
if (prev_hook)
_prev_hook;
else
_standard_ProcessUtility;

@jkatz jkatz merged commit 6685795 into aws:main Jul 3, 2023
@leopan3 leopan3 deleted the issue208 branch July 5, 2023 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants