Skip to content
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

fix(mssql): avoid calling .commit() unless a DDL operation is being performed #9658

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Jul 22, 2024

This PR attempts to address flaky behavior of MS SQL observed in CI.

It seems like calling commit()/rollback() with a SELECT statement, AKA autocommit behavior
can cause this issue.

The origin is not 100% clear to me, but there are a number of places where the
same problem (function sequence error) with the same solution show up
(disabling autocommit or not calling commit with a SELECT statement):

Here I avoid calling cur.commit() unless a DDL statement is being executed.

The remaining case is raw_sql(), which I opted to avoid calling commit in
since that would have this problem when calling it with a SELECT statement.

Users of raw_sql are therefore responsible for handling commit/rollback when
invoking it for DDL statements.

Closes #9654.

@cpcloud cpcloud added bug Incorrect behavior inside of ibis mssql The Microsoft SQL Server backend labels Jul 22, 2024
@cpcloud cpcloud requested a review from gforsyth July 22, 2024 16:20
Copy link
Member

@gforsyth gforsyth left a comment

Choose a reason for hiding this comment

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

thanks for tracking this down!

:shipit:

@gforsyth gforsyth merged commit 69c5bf0 into ibis-project:main Jul 22, 2024
87 checks passed
@cpcloud cpcloud deleted the dont-use-commit-in-select-mssql branch July 22, 2024 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior inside of ibis mssql The Microsoft SQL Server backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test(mssql): flaky test around dataframe interchange protocol
2 participants