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

feat: add transaction related method #360

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

hantmac
Copy link
Member

@hantmac hantmac commented Mar 6, 2024

  • Add begin, commit, rollback transaction method.

@hantmac
Copy link
Member Author

hantmac commented Mar 12, 2024

So strange, I run cargo clippy --all-targets -- -D warnings successfully on local machine, but the ci failed https://github.com/datafuselabs/bendsql/actions/runs/8242790623/job/22542294683?pr=360#step:4:586

@hantmac hantmac force-pushed the feat/add-transaction-method branch from 48d0780 to 17da5f4 Compare March 12, 2024 02:47
driver/src/flight_sql.rs Outdated Show resolved Hide resolved
driver/src/conn.rs Outdated Show resolved Hide resolved
sql/src/value.rs Outdated Show resolved Hide resolved
@hantmac hantmac force-pushed the feat/add-transaction-method branch from 78d279f to 60a1b79 Compare March 19, 2024 06:47
@@ -190,6 +190,21 @@ pub trait Connection: DynClone + Send + Sync {
))
}

async fn begin(&self) -> Result<()> {
let _ = self.exec("BEGIN").await;
Copy link
Member

Choose a reason for hiding this comment

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

errors should be handled?

@hantmac hantmac force-pushed the feat/add-transaction-method branch from 2be8397 to f6e3db8 Compare March 21, 2024 03:09
let (val,): (i32,) = row.try_into().unwrap();
assert_eq!(val, 1);

conn.rollback().await.unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Should recheck after rollback?

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.

None yet

3 participants