Skip to content

DATAJDBC-356 - Support for reading large resultsets #903

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

Closed

Conversation

denniseffing
Copy link

Support for reading large result sets using the new queryForStream methods in Spring Framework 5.3.

Copy link
Contributor

@schauder schauder left a comment

Choose a reason for hiding this comment

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

Excellent PR.
I have just the single request to change the behaviour in the presence of a ResultSetExtractor.

As another side note: The "Closes" and "Original pull request" should reference the github issue number and the number of this PR.
But I'm happy to add these when merging

@schauder schauder added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 15, 2021
@gregturn gregturn changed the base branch from master to main April 15, 2021 18:21
@denniseffing denniseffing force-pushed the issue/DATAJDBC-356 branch 2 times, most recently from 182faef to b24cb44 Compare June 28, 2021 10:46
@denniseffing
Copy link
Author

@schauder I finally got to finish this up!
I rebased the PR onto the current main branch and incorporated your suggestion to fall back to the existing behavior when a ResultSetExtractor is specified. Please let me know if you require any additional changes.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 28, 2021
Use queryForStream for streamed query results.
ResultSetExtractor is ignored because it cannot be used together with streams.

Closes spring-projects#356
Specifying a custom result set extractor doesn't make sense when using
streams because the result set extractor always considers the entire
result. This loads the entire stream into memory anyways.
@denniseffing denniseffing requested a review from schauder June 28, 2021 11:48
@schauder
Copy link
Contributor

schauder commented Jul 2, 2021

Thanks.
I added some documentation and merged this PR.

@schauder schauder closed this Jul 2, 2021
@schauder schauder linked an issue Jul 2, 2021 that may be closed by this pull request
schauder pushed a commit that referenced this pull request Jul 2, 2021
Use queryForStream for streamed query results.
Since ResultSetExtractor cannot be reasonably be used together with streams it falls back to the existing collection behaviour.

Closes #578
Original pull request #903
schauder added a commit that referenced this pull request Jul 2, 2021
Also us Github issue numbers on tests.

Original pull request #903
See #578, #971
schauder added a commit that referenced this pull request Jul 2, 2021
schauder added a commit that referenced this pull request Jul 5, 2021
As suggested by Jay Bryant.

Original pull request #903
See #578, #971
@denniseffing denniseffing deleted the issue/DATAJDBC-356 branch January 4, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided Feedback has been provided
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for reading large resultsets [DATAJDBC-356]
3 participants