Faithlife / FaithlifeData

Helpers for querying ADO.NET-compatible databases.
https://faithlife.github.io/FaithlifeData/
MIT License
6 stars 4 forks source link

Support cached commands. #11

Closed ejball closed 4 years ago

ejball commented 4 years ago

Using Prepare() Cache() on DbConnectorCommand causes Prepare() to be called on the IDbCommand. More importantly, it caches the command and reuses the command object (when using the same command text) for the lifetime of the connector. The Faithlife SQLite connector in particular can take advantage of this because preparing the command creating the native command (i.e. parsing the SQL) can be relatively expensive.

bgrainger commented 4 years ago

Using Prepare() on DbConnectorCommand [calls IDbCommand.Prepare and caches the command]

Interesting design choice to conflate those two behaviours. It probably doesn't matter in practice because SQLite doesn't support prepared commands (AFAIK) and MySqlConnector has IgnorePrepare=true by default. Still, it does seem a little odd to have two distinct side-effects (of the call to Prepare) that can't be separated.

ejball commented 4 years ago

Interesting design choice to conflate those two behaviours.

True. Maybe I should change Prepare() to Cache() and call it good, especially since we don't know any providers that encourage preparing commands.

bgrainger commented 4 years ago

I commented on an outdated line of code, so maybe you didn't see it:

What's the purpose of the PreparedCommand class? I'm having difficulty figuring out why it exists.

ejball commented 4 years ago

What's the purpose of the PreparedCommand class? I'm having difficulty figuring out why it exists.

Now CachedCommand, it wraps the actual IDbCommand to prevent it being disposed.

bgrainger commented 4 years ago

it wraps the actual IDbCommand to prevent it being disposed

Now that you've told me this, I can see that Dispose is empty (and does not delegate), but I stared at it for the longest time trying to figure it out. Please add a comment to Dispose or to the class itself giving the rationale for its existence.

ejball commented 4 years ago

Please add a comment ... to the class itself

Done.