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 for statement caching issue #191

Closed
wants to merge 1 commit into from
Closed

Conversation

TomSwift
Copy link

This is a proposed fix for the statement caching issue described in #6

My approach is basically what @mattstevens proposes in the issue thread: Cached statements are now only dolled out when they aren't currently associated with an open FMResultSet. Once an associated result set is closed the cached statement becomes available for reuse.

I added a simple test case to fmdb.m to validate the fix and confirm that it doesn't break statement caching altogether. All tests in fmdb.m apparently continue to pass. I ran no other regression tests to see if I somehow broke something else.

My fix uses __weak, but I provided a non-arc/non-weak compilation path. But this is untested.

I believe this fix negates the need for the fix applied for similar issue #106. But that fix doesn't conflict with mine.

only use a cached statement if it is not currently associated with a result-set.

automatically associate/disassociate statements/result-sets.

basic test case in fmdb.m
@ccgus
Copy link
Owner

ccgus commented Oct 15, 2013

Thanks for this- I'll hopefully get a chance to review + commit it soon.

@ccgus
Copy link
Owner

ccgus commented Oct 22, 2013

Hola! I've taken your patches and reworked them a bit - and they are now committed on the main branch. Thanks for fixing this!

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