Skip to content

Conversation

@deniskuzZ
Copy link
Member

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

@deniskuzZ deniskuzZ changed the title Hive 25346 HIVE-25346: cleanTxnToWriteIdTable breaks SNAPSHOT isolation Oct 12, 2021
@deniskuzZ deniskuzZ force-pushed the HIVE-25346 branch 3 times, most recently from 0d432d5 to 3f1392b Compare October 12, 2021 17:50

} catch (Exception e) {
throw new MetaException(StringUtils.stringifyException(e));
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use try-with-resorces here for the statement and the dbConnection?

Copy link
Member Author

Choose a reason for hiding this comment

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

we could, however, it would introduce 1 more level of nesting, I would prefer to keep it like this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe wrong, but the finally is only there for closing the dbConn and the stmt. We can skip that with try-with-resources.

Copy link
Member Author

Choose a reason for hiding this comment

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

however, that would introduce few more levels of nesting: +2

Copy link
Member Author

Choose a reason for hiding this comment

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

refactored

…ces, removed redundant dbConn in checkRetryable
@deniskuzZ deniskuzZ merged commit 5bd9a14 into apache:master Oct 20, 2021
@deniskuzZ deniskuzZ deleted the HIVE-25346 branch October 20, 2021 10:09
HarshitGupta11 pushed a commit to HarshitGupta11/hive that referenced this pull request Dec 12, 2021
dengzhhu653 pushed a commit to dengzhhu653/hive that referenced this pull request Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants