QueryBuilder Stack Overflow Issue

[terryaney]

[UPDATE] Simplifying issue from previous iteration and will investigate my previous issues further once this is fixed.

QueryBuilder is crashing NUnit with stack overflow exception while SqlBuilder works.

// NUnit crashes
var qb = new QueryBuilder(@$"
	SELECT COUNT(*) TotalCount 
	FROM [File] f 
		INNER JOIN Folder fo ON f.Folder = fo.UID 
	WHERE f.Deleted != 1 /**filters**/
");

// NUnit is able to run test
var qb = new SqlBuilder(@$"
	SELECT COUNT(*) TotalCount 
	FROM [File] f 
		INNER JOIN Folder fo ON f.Folder = fo.UID 
	WHERE f.Deleted != 1 /**filters**/
");

Trying to wrap my head around code, but here is the overflow:

QueryBuilder.Build ->
InterpolatedSqlBuilderBase.AsSql (this is ISqlEnricher enricher is true) ->
QueryBuilder<U, RB, R>.GetEnrichedSql ->
[starts cycle over]

        // QueryBuilder
        public override IInterpolatedSql Build()
        {
            return AsSql();
        }
        // InterpolatedSqlBuilderBase
        public IInterpolatedSql AsSql()
        {
            if (this is ISqlEnricher enricher)
                return enricher.GetEnrichedSql();
            return new SqlBuilderWrapper(this);
        }
        // QueryBuilder<U, RB, R>
        IInterpolatedSql ISqlEnricher.GetEnrichedSql()
        {
            return Build();
        }

[Drizin]

Yeah, the class design is more complicated than I'd like it to be. The general idea is:

• All builder types should be extensible using recursive generics, and they all should support a Build() method.
• Any builder that has custom-logic to build the final SQL statement (i.e. it does more than simple concatenation) should implement ISqlEnricher. It can even inherit from SqlBuilder (so it can mostly still be used like a simple concatenation builder) but by implementing ISqlEnricher it can make minor transformations to the generated SQL (QueryBuilder is an example - it's exactly like SqlBuilder but when rendered it makes some minor transformations like replacing the /**where**/).
• In most cases GetEnrichedSql is just a facade to call the regular Build() (in other words Build() also renders the same custom SQL. Which means that maybe this interface is just redundant)

The stack overflow happens when you use the non-generic InterpolatedSql.SqlBuilders.QueryBuilder, right? If so, I think I found and fixed the problem:

Looks like the generic types (like InterpolatedSql.SqlBuilders.QueryBuilder<U, RB, R>) have an implementation of R Build() that does NOT invoke AsSql() on themselves, so they don't have the recursion problem. It also doesn't affect subtypes like InterpolatedSql.Dapper.SqlBuilders.QueryBuilder).

However the non-generic subtype InterpolatedSql.SqlBuilders.QueryBuilder has the problem since it overrides IInterpolatedSql Build() and this method goes recursively, like you've noticed. This override probably shouldn't be there - I think I copied that by mistake from InterpolatedSql.SqlBuilder.

Let me know if my last commit fixes the problem.

And thanks again for the detailed report, as usual.

[terryaney]

Yes, that seemed to fix it. So, DapperQueryBuilder supported multiple occurrences of /**filters**/ in a query and it replaced all of them. The real problem I'm chasing now is why InterpolatedSql.Dapper does not.

I'm digging, but the following two pieces of code look basically identical to me...

DapperQueryBuilder/QueryBuilder.cs

InterpolatedSql.Dapper/QueryBuilder<U, RB, R>

[Drizin]

I think that Major Rewrite (DapperQueryBuilder 2.0) was never published.
So probably the code you have is using this Replace():
Drizin/DapperQueryBuilder@93ebb16#diff-f006871c155dfe920194edcc0d3ac95c5f1976efbc0f5373382fe18cee428005R137

Feel free to modify the code to allow multiple matches. I hadn't thought about this use-case before but it makes sense. You'll probably want to run backwards so that .Remove() and .Insert() won't change the index positions of the previous matches.

[terryaney]

Working on it...created PRs in InterpolatedSql and DapperQueryBuilder, but need your help running tests since I don't have all databases installed. Closing this issue.