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

Proposal: Add BufferResultSets Connection String Option #202

Closed
caleblloyd opened this issue Mar 21, 2017 · 4 comments
Closed

Proposal: Add BufferResultSets Connection String Option #202

caleblloyd opened this issue Mar 21, 2017 · 4 comments
Assignees

Comments

@caleblloyd
Copy link
Contributor

@caleblloyd caleblloyd commented Mar 21, 2017

I'm proposing a new connection string option named BufferResultSets that will read all Result Sets into memory upon calling ExecuteReader and ExecuteReaderAsync. It would default to false and would have to be explicitly enabled.

The intended use case is for ORMs that need to support the SQL Server-like feature Multiple Active Result Sets. MySQL does not support MARS, but by buffering all result sets and clearing the active reader, the connection will be open to service another reader. Since ORMs typically end up reading all results into memory anyways, this should not add much extra memory overhead.

There is a couple ways this could be implemented:

  1. Simple: If BufferResultSets is set, immediately buffer all result sets into memory upon calling ExecuteReaderAsync
  2. Complex: If a connection receives a call while there is an active reader, first buffer the active reader into memory in order to clear the reader, then service the next call

Logic to buffer result sets already exists in MySqlClient/Results/ResultSet.cs

This option would remove the need for SynchronizedMySqlDataReader.cs in Pomelo.EntityFrameworkCore.MySql.

@bgrainger @kagamine

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Mar 21, 2017

Since ORMs typically end up reading all results into memory anyways, this should not add much extra memory overhead.

Dapper buffers all results by default and claims that it's "ideal in most cases".

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Mar 21, 2017

No objections to this proposal. The "Simple" approach sounds best to me; it doesn't seem like a bad idea to buffer the data immediately if the user has opted-in to it.

@caleblloyd caleblloyd self-assigned this Mar 22, 2017
@caleblloyd
Copy link
Contributor Author

@caleblloyd caleblloyd commented Mar 22, 2017

I've begun work on the "Simple" approach here: f_bufferResultSets

@bgrainger
Copy link
Member

@bgrainger bgrainger commented Mar 22, 2017

Implemented in 0.15.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants