Translating an inputted query to SQL

I'm working on a small RestApi that would be able to:

• Interpret different query languages like (...)/countries?where="..."&select="..." or (...)/GetCountries?ids="1,2,3"

• Translate theses queries to SQL

• Pass the SQL command to dapper and return the result

Note this API will be used by other services and that it won't be exposed to the "public".

I have a first version running but I'm not satisfied with some aspects of my solution...

So how does it work?
When the rest api (ASP.NET CORE) receives a query, it will try to find a parser able to interpret it. Currently I have only one simple parser.

```
public interface IQueryParser
{
Query Parse(QueryContext context);
}

public class DqlQueryParser : IQueryParser
{
private readonly Func _predicate;
private readonly IQueryOperatorParser _parser;

public DqlQueryParser(Func predicate, IQueryOperatorParser parser)
{
_predicate = predicate ?? (x => true);
_parser = parser;
}

public Query Parse(QueryContext context)
{
if (context == null)
throw new ArgumentNullException(nameof(context));
if (!string.Equals(context.QueryLanguage, "dql", StringComparison.OrdinalIgnoreCase) || !_predicate(context))
return null;
if (string.IsNullOrEmpty(context.Query))
throw new QueryParserException(context, "The supplied query context does not contain a valid query string. It shouldn't empty or null");

var type = context.Query.GetEntityType();
if (type == null) throw new QueryParserException(context, $"The supplied '{context.Query}' entity is missing or unknown");

switch (context.Method)
{
case "GET":
return new DqlSelectQuery(context.Query, type, _parser?.Parse(context) ?? Enumerable.Empty());
case "POST":
return new DqlInsertQuery(context.Query, type, (JObject)JsonConvert.DeserializeObject(context.Body));
case "DELETE":

A proper code review would need more details about your design, especially about DqlSelectQuery (especially purpose and intended use), then I'd limit myself to few things in your visible code.

First things I'd consider to change is string interpolation. In this moment you're, in my opinion, abusing this feature. It's very handy for short interpolated expressions but you're extensively using this language feature with long expressions and it's killing readability. Don't go to refactor this now...

Major problem is you're building SQL code by hand. Biggest problem is that it's completely open to SQL injection (for example check WHERE, OFFSET and FETCH NEXT). As you can imagine this is a terribly bad thing. OFFSET and FETCH NEXT are integers and they may be sanitized however I don't like to reinvent the wheel (with high chances to make it wrong). What I'd like to return from a function like this is the SQL command text plus an object that can be used for SQL parameters (in conjunction with a mapper, for example Dapper). Something like this:

dynamic parameters = new ExpandoObject();
if (ops.AnyOfType())
{
    parameters.Offset = ops.FirstOfType().Offset;
    sb.AppendLine("OFFSET @Offset ROWS");
}

if (ops.AnyOfType())
{
    parameters.Limit = ops.FirstOfType().Limit;
    sb.AppendLine("FETCH NEXT @Limit ROWS ONLY");
}

For WHERE clause you'd better use the same approach of EF and go with Expression, it's a big step in complexity but much easier to use at calling point and much more safe. You may like to read Building LINQ Queries at Runtime in C# for inspiration. After this first step I'd split function into multiple parts, each one with a single responsibility. If you like to give names you may call it chain of responsibilities. It may be as easy as multiple private functions invoked through Reflection or a more complex mechanism. For example:

void OperatorOffset(StringBuilder sql, dynamic parameters)
{
    if (!_query.Operators.AnyOfType())
        return;

    parameters.Offset = ops.FirstOfType().Offset;
    sql.AppendLine("OFFSET @Offset ROWS");
}

Note that _query is an instance field (the parameter of Translate() method). Now I'd like to address another problem: you're roaming around with a SQL string and errors will be catched only at run-time. Multiple places with string concatenation means that you have to test every single possible code path with all possible relevant values. Nice to have but I'd like to make it little bit more safe! You may introduce a command builder class, it's pretty handy especially for JOIN and SELECT:

var command = new CommandBuilder();

if (query.Operators.AnyOfType())
{
    command.AddSelect(ops.FirstOfType().Fields,
        query.TargetEntityType.Name);
}
else
{
    command.AddSelect(query.TargetEntityType.Name);
}

Where AddSelect() may be something like this:

private const string SqlCommandSelect = "SELECT";
private const string SqlFromClause = "FROM";

public CommandBuilder AddSelect(IEnumerable fields, string tableName)
{
    _sql.AppendLine(SqlCommandSelect)
        .AppendLine(String.Join(",\n", fields.Select(x => QuoteIdentifier(x)))
        .AppendLine($"{SqlFromClause} {QuoteIdentifier(tableName)}");

    return this;
}

Of course code is not complete but I guess you get the point. Note the call to QuoteIdentifier() (to be written), identifiers must be properly quoted! The two overloaded versions may even be merged (if (fields?.Count()) ?? 0 == 0 then just add *). Fields may be prefixed with table alias (useful in case of complex joins to avoid ambiguity.)

Now that you have a CommandBuilder class you can move SQL parameters inside it and your Translate() function will simply return that object. You may add handy helper methods to transform this command into a SqlCommand (SQL command text + parameters) or to a plain SQL string (for debugging purposes). If you know your building a SqlCommand (or similar) you can directly add parameters there, no need for an ExpandObject or a Dictionary. Something similar, but with completely different purpose, can be seen in SqlCommandBuilder class.

I think you're almost done but there is still a thing that hurts me. All those AnyOfType() and FirstOfType(). Can you specify more than one DqlSelectOperator operator? Isn't it an error? Yes, it is then caller must be warned of this. You introduced your own FirstOfType() extension method then also add SingleOfType() (or use plain syntax .OfType().Single()): if query is broken then it won't silently do something (probably unexpected).

Last note: things may be more complicate if you're using this interface to abstract queries to different repositories (let's say you have a query language for relational databases, NoSQL databases, XML databases/files, JSON files, etc...) In this caseyou need to return ICommandBuilder and you will need few specializations (one for each target format):

```
publ

StackExchange Code Review Q#149720, answer score: 3