Lack of Connection Dispose in documentation

[joshcobalt]

The documentation is lacking any usage of .Dispose() (or wrapping with a using() block) on the instantiated sql connections. e.g. https://sqlkata.com/docs/execution/setup

Is this being called somewhere in the code for you that I'm not seeing?

Otherwise, the documentation/code should be updated to suggest a better way of managing the life-cycle of the connection object. Many people will just be copy/pasting and could end up exhausting the database connections.

[ahmad-moussawi]

While we try to keep it as simple as possible, you have a point here, we should keep a note for people that are not using a container-based framework, to dispose the connection manually.

What framework (web/console/mvc..) are you using SqlKata with?

[joshcobalt]

I'm using it via asp.net mvc, but it applies to all use cases.

I think if the QueryFactory is changed to implement IDisposable and disposes the internal IDBConnection, then the IOC container will naturally dispose the connection when that thread/request completes. https://docs.microsoft.com/en-us/aspnet/core/fundamentals/dependency-injection?view=aspnetcore-2.2#disposal-of-services

Thoughts?

[madben2000]

Any progress on that topic? I am also searching for a way to dispose the connection. I'm using SqlKata on an Asp.Net MVC application (at the moment without DI container)

[benmccallum]

I'm getting connection pool exhaustion and this is the likely culprit. Partly my own fault, for new'ing up a SqlConnection and calling it a day, but it would be good to have a best practice here or some docs.

I'd be for @joshcobalt 's suggestion to make the QueryFactory implement IDisposable and disposes the DbConnection it is given then it'd be easier to not make mistakes on this. I guess that's probably what EF does with their DbContext, but would need to research.

My only concern would be that since the IDbConnection is given to the QueryFactory, it really should be the caller who gave it that manages its lifetime. So perhaps documentation is the way to go.

[markmaior]

Hi everybody, I try to resolve the problem implementing a class like this:

public class QFactory : IDisposable
{
        public QueryFactory Factory { get; set; }

        public QFactory()
        {
            Factory = new QueryFactory(new SqlConnection("conn"), new SqlServerCompiler());
        }

        /// <inheritdoc />
        public void Dispose()
        {
            Dispose(Factory.Connection.State == ConnectionState.Open);
            GC.SuppressFinalize(this);
        }

        protected virtual void Dispose(bool disposing)
        {
            if (disposing)
                Factory.Connection.Close();
        }
    }

So, when I need to query the DB I write code like this:

using (var dbFactory = new QFactory())
            {
                var activeCustomers = dbFactory.Factory.Query("Customers").Where("ACTIVE", 1).Select("ID");
            }

This could be a good way to resolve the connection pool exhaustion problem?

[benmccallum]

I think one of the biggest issues is the docs just doesn't say you need to dispose it. If the ctor for QueryFactory takes a DbConnection, well yea, the caller needs to manage it, fair enough. But:

1. The readme under Setup Connection just new's up a SqlConnection without highlighting it'd need to be disposed.
2. The docs online under Query Factory should a similar story
3. The asp.net core example with DI is similar, but even worse, in that it looks like that's all you'd have to do, and the DI container, when it deconstructs the QueryFactory will just dispose the connection for you, but as we know, that's not the case.

All of these places could do with better warnings/examples, or QueryFactory implements IDisposable and that is the documented pattern.

[ahmad-moussawi]

Thank you all for your feedback, I agree that QueryFactory should implements IDisposable, and this WOP under https://github.com/sqlkata/querybuilder/blob/feature/idisposable/SqlKata.Execution/QueryFactory.cs.

For the documentation specific issues I appreciate if you can open it under https://github.com/sqlkata/docs/issues/new

[JunRamoneda]

Hi, any timeframe on when disposable QueryFactory will be included in a release?

[ahmad-moussawi]

Yes many enhancements are planned to be included in the next release, estimated time on 15 May. The most two major enhancements are, Disposable implementation + Proper Transaction Support. Some enhancements that am working on (no precise timeline yet) is the JSON support

[sarvasana]

Tomorrow is May 15th. Will the disposable QueryFactory be there tomorrow? I am aware you did not mention a century....

[ghost]

Any updates?

[ahmad-moussawi]

Yes :) actually the QueryFactory class implements disposable now, you can get it on from the latest official version, the documentation will be updated soon.