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: New sqliteIsTransacting() that returns if a transaction is active on the current connection #462

Merged
merged 9 commits into from Jun 8, 2023

Conversation

bpvgoncalves
Copy link
Contributor

In response to issue #453 I implemented a functionality to allow checking of transaction status.
I cannot find any develop branch so the PR targets the main branch.

It passes the tests:

══ Results ═════════════════════════════════════════════════════════════════════════════════════════════════════════
Duration: 35.5 s

── Skipped tests  ──────────────────────────────────────────────────────────────────────────────────────────────────
• astyle not found (1)
• No schemas available (1)
• NYI (1)
• On Linux (2)
• tweak: !date_typed (8)
• tweak: !time_typed (5)
• tweak: !timestamp_typed (8)

[ FAIL 0 | WARN 3 | SKIP 26 | PASS 7476 ]

It CMD CHECKs in my local machine (only 1 NOTE unrelated to the changes)

── R CMD check results ───────────────────────────────────────────────────────────────────── RSQLite 2.3.1.9002 ────
Duration: 2m 11s

❯ checking compilation flags used ... NOTE
  Compilation used the following non-portable flag(s):
    ‘-Wformat’ ‘-Wp,-D_FORTIFY_SOURCE=2’

0 errors ✔ | 0 warnings ✔ | 1 note ✖

R CMD check succeeded

Best Regards

Copy link
Member

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Great, thanks!

R/transactions.R Outdated Show resolved Hide resolved
src/connection.cpp Outdated Show resolved Hide resolved
@krlmlr krlmlr changed the title Feature: transaction state feat: New sqliteIsTransacting() that returns if a transaction is active on the current connection Jun 8, 2023
@krlmlr krlmlr merged commit 72e5d4e into r-dbi:main Jun 8, 2023
12 checks passed
@krlmlr
Copy link
Member

krlmlr commented Jun 8, 2023

Thanks!

@bpvgoncalves bpvgoncalves deleted the feat_transaction-state branch June 8, 2023 18:49
@krlmlr
Copy link
Member

krlmlr commented Jun 12, 2023

@bpvgoncalves: 2d4ac14 introduces a problem with the orderly package, found through git bisect. That package fails R CMD check after this commit.

Other packages fail too with the current main branch. Can you please take a look?

My test.sh for git bisect :

#!/bin/sh

set -ex

git clean -fdx src
R CMD INSTALL .
R -q -e 'rcmdcheck::rcmdcheck("~/git/R/orderly")' 

@krlmlr krlmlr mentioned this pull request Jun 12, 2023
3 tasks
@bpvgoncalves
Copy link
Contributor Author

bpvgoncalves commented Jun 13, 2023 via email

@bpvgoncalves
Copy link
Contributor Author

So... It looks orderly, and possibly other packages, are at some point applying a commit/rollback when no open transactions exists.
The new functions did not allow this behavior by raising an error.

After applying PR #464 it seem to be fixed.
When testing 'orderly' locally I still receive some errors when R CMD check'ing due failure in latex->pdf conversion for docs. devtools::check(document = FALSE) completes successfully.

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

2 participants