querybuilder icon indicating copy to clipboard operation
querybuilder copied to clipboard

Add ability to extend Query class

Open jhm-ciberman opened this issue 2 years ago • 7 comments

Currently, all the methods in the Query class return an instance of Query.

At the project we are working on, we have a ModelQuery<TModel> that have useful methods for retrieving Models in our database, handling relationships, soft deleting, eager loading, etc. (Similar to the eloquent query builder in the Laravel Framework)

We have two options:

Option one: If we make our ModelQuery<TModel> class extend BaseQuery<ModelQuery<TModel>> then all the Where methods are chainable since they correctly return a ModelQuery<TModel>. But we loose access to all the other methods like OrderBy, Take, Limit, Select, etc.

Option two: Make our ModelQuery<TModel> extend the Query class. We have access to the full toolbox of methods, but we loose the ability to chain with our own methods, since all methods in the Query class returns a Query object, not a generic Q object like the BaseQuery<Q> class.

Proposed solution: The solution would be to have an intermediate abstract class SqlKataQuery<Q> that extends BaseQuery<Q> but have all the current implementation of the Query class but each method return a generic Q object.

In this way, the inheritance chain would look like:

abstract class BaseQuery<Q> where Q : BaseQuery<Q>
{ 
    // This class would contain all the Where(..) methods
}

abstract class SqlKataQuery<Q> : BaseQuery<Q>  where Q : SqlKataQuery<Q>
{ 
    // All the current implementation of the Query class would go 
    // here but returning a generic Q object 
}

class Query : SqlKataQuery<Query>
{
    // This class will be concrete, not abstract, and will have no 
    // implementation because all the methods will be in the base class
}

In this way, anyone can extend SqlKataQuery to create their own unique specialized query builders. For example:

class MyCustomQuery : SqlKataQuery<MyCustomQuery>
{

    public MyCustomQuery DoMagic()
    {
        // Add a magic component to the query
        return this;
    }
}

I can make a PR if you like. What do you think?

jhm-ciberman avatar Sep 17 '21 18:09 jhm-ciberman

Hi @jhm-ciberman, I am interested in such feature, let me know if you are still ready for a PR so maybe we can discuss this further.

ahmad-moussawi avatar Feb 10 '22 21:02 ahmad-moussawi

I think this would be a great feature, I'm defining some classes at the moment to help dynamically build some reports and I'm having to cast the return type for every one of my new methods.

I'm not sure what happened to @jhm-ciberman but I'll investigate implementing this and open a new Issue/PR when done.

Meigs2 avatar Oct 10 '22 16:10 Meigs2

Hi! I'm still alive. I just don't have enough time to make the PR at the moment. I'm the only developer assigned to the current software we are making at work so I'm finishing it just casting everything and doing it the "dirty" way.

But it would be nice to have this feature implemented.

jhm-ciberman avatar Oct 10 '22 19:10 jhm-ciberman

After some initial investigation it seems like this would take a bit of refactoring, first steps seem to be to make sure that all uses of the usual Query class call the proper NewQuery overload, as several places in the query builder simply call new Query(). Also, a few conditionals directly use Query instead of T : BaseQuery<T>.

EDIT: Actually most of everything from the Compiler to the SqlResult need to be genericized to accept a SqlKataQuery<T> instead of working off a Query directly.

Meigs2 avatar Oct 11 '22 14:10 Meigs2

I think something else interesting that could be added is a sort of "blank canvas" query class you could derive from that does not expose any of the usual methods for building the query, instead accessing protected methods exposed by the SqlKataQueryBase, and an interface like ISqlKataQuery which defines the methods that are exposed publically on the Query class.

The use case would be when you want to limit or control what the consumer of a query can do/access. For example, if you are creating a SalesReportQuery you might want to be able to re-use and limit how the query is built. Sample:

// Existing base query class
public class BaseQuery<T>{}
    
// Definition of internally required query methods
public abstract class SqlKataQueryBase<T> : BaseQuery<T> where T : SqlKataQueryBase<T>
{
    protected internal abstract T WhereInternal(string column, string op, object value); // protected internal
    // ... other methods
}

// Base class for all queries, used to compile, etc. Current `Query` class.
public abstract class SqlKataQuery<T> : SqlKataQueryBase<T> where T : SqlKataQuery<T>
{
    // marked protected so that only derived classes can call it
    /// <inheritdoc />
    protected internal override T WhereInternal(string column, string op, object value)
    {
        // ... implementation
    }
}

// Derived class from SqlKataQuery does not expose public methods from ISqlKataQuery, restricting
// how the query can be used and built.
public class SalesReportQuery: SqlKataQuery<SalesReportQuery>
{
    public SalesReportQuery() : base("Table AS table")
    {
    }
    
    // This would be the Only publicly exposed method, user-defined control over building this specific query
    public SalesReportQuery OrdersAfter(DateTime date)
    {
        // Where is defined in the base class, protected.
        return WhereInternal("table.Date", ">=", date);
    }
    
    // ... other methods which define the query
}

// Definitions of current public methods
public interface ISqlkataQuery<T> where T : SqlKataQuery<T>
{
    T Where(string column, string op, object value);
}

// Derived class from SqlKataQueryBase *does* expose public methods from ISqlKataQuery, allowing for more flexibility
// with the query builder and custom methods. This would take the place of "SqlKataQuery" in the issue proposal.
public class Query<T> : SqlKataQuery<T>, ISqlkataQuery<T> where T : SqlKataQuery<T> // inherits to ensure we expose public facing methods
{
    // "exposes" the internal method.
    public T Where(string column, string op, object value)
    {
        return WhereInternal(column, op, value);
    }
}

// This would be the standard Query class, backwards compatible with existing code.
public class Query : SqlKataQuery<Query>
{
}

This also opens the door for adding things like pre-compilation event hooks to conditionally add joins, where's, etc depending on how the user called the base class. For example, in a report query I wrote recently, several different filters touched the same table at different points, and I had to keep internal track of when I needed to add the join to the final query so I could efficiently re-use the same query to filter down results. I added a Build() method that checked if the subquery was ever built and joined it at the end to the main query. However, a user of my custom class would not know to call "Build" to get the correct query before compilation. You could define a "builder" interface, but a hook mechanism might enable some additional behaviors before compilation.

Thoughts? So long as the API does not change, there should not be any issues with backwards compatibility.

Meigs2 avatar Oct 11 '22 21:10 Meigs2

Usually in these cases I prefer composition over inheritance, I can think of your example as the following

public class SalesReportQuery
{
    protected Query query;

    public SalesReportQuery(Query query)
    {
        this.query = query.From("Sales");
    }
    
    public Query OrdersAfter(DateTime date)
    {        
        return query.Where("table.Date", ">=", date);
    }    
}

I find this simpler, but I may missed the key points of your proposal

ahmad-moussawi avatar Oct 12 '22 09:10 ahmad-moussawi

That solution works fine, it's probably what I'll use going forward. All just food for thought.

I'm currently working on implementing the item in the issue, thoughts on the naming scheme? SqlKataQuery<Q> for the intermediate type? Query<Q>?

Meigs2 avatar Oct 12 '22 12:10 Meigs2