added reader #120
Merged
added reader #120
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@joeferner There was a serious memory/cursor leak in my original PR (reader was never destructed). I have fixed it and tested both with 0.10.x and 0.11.10. Let me know what you want to do with it. If you have better ideas for the API, we can discuss here and I'll adjust and resubmit the PR. |
|
@joeferner Thanks for accepting the PR. I'm testing more and I'm running into a segfault with a program that I had not run before. I found a simple solution which avoids the Sorry for that. I'm still on the learning curve with the V8's APIs. Bruno |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a first pass on a reader API. I've added a section to the README.
I've tried to disturb the existing code as little as possible. There is some overlap between
Connection::ExecuteandReader::NextRowsso the code could be refactored a bit.I have added a unit test and also tested with our application. It seems to work but here may be memory leaks. I'm going to stress it more in the coming days.
I've also added a few lines of code to map
RAWandROWIDtypes. This is only temporary (RAWshould probably be mapped toBufferrather thanstring) but it helped me test with our app.Note: this is a reader rather than a cursor because there is no API to act on a current record. The records are read with a window, which optimizes for cases where we scan large results sets from beginning to end.