sqlkata / querybuilder

SQL query builder, written in c#, helps you build complex queries easily, supports SqlServer, MySql, PostgreSql, Oracle, Sqlite and Firebird
https://sqlkata.com
MIT License
3.08k stars 498 forks source link

Basic problems with generated SQL for Postgres #627

Closed MortenChristiansen closed 1 year ago

MortenChristiansen commented 1 year ago

I have a feeling that I'm missing something, but my experience with the PostgresCompiler is that it has several problems creating valid queries even for simple scenarios. There are things which seem so basic that I cannot be the first to run into them - hence the feeling that I am missing something. The queries were executed by manually taking the compiled sql and executing them with Dapper, if it makes any difference. I'm using version 2.4 of the library.

1) When using query.WhereContains, the query string itself is not surrounded with 's which it must be. I updated the CompileBasicStringCondition implementation to change the switch statement to the following:

switch (x.Operator)
{
    case "starts":
        text2 = "'" + text2 + "%'";
        break;
    case "ends":
        text2 = "'%" + text2 + "'";
        break;
    case "contains":
        text2 = "'%" + text2 + "%'";
        break;
}

2) When using query.WhereLike I had the same problem as in 1), but here the solution was just to prewrap my argument in single quotes at the call site. This is less of a big deal, but takes away from a feeling that the library "just works".

3) When selecting columns, they are wrapped in "s. This is not valid, so I had to do the following in my compiler.

public override string WrapValue(string value) =>
    value;

I'm not sure if this has any influence on 1) and 2), but I don't think so since the quotation character is " which is not what is needed in those cases.

ahmad-moussawi commented 1 year ago

How are you executing your query?,are you using the output of SqlResult.ToString() method?

MortenChristiansen commented 1 year ago

@ahmad-moussawi I'm compiling like this and then executing the sql with Dapper. There may be one or more queries in the array. var sql = compiler.Compile(arrayOfQueries).Sql

ahmad-moussawi commented 1 year ago

Can you please confirm if you face the issue when using the Compile(Query) overload?

MortenChristiansen commented 1 year ago

Yes, this makes no difference.

MortenChristiansen commented 1 year ago

I started building a minimal repro, when I realized something. I used the search parameter expecting it to be inserted directly. However, the library actually treats it as a query parameter which I did not expect. As a query parameter, it makes sense that it would not be quoted I think (I'm not sure if SELECT * FROM Users WHERE Username LIKE %@p0% is valid or not, but I expect it is). It makes sense that it would not just inject the search query verbatim since that would facilitate injection attacks - I just didn't realize it. I'll have to look into why my code worked regardless which is a bit weird.

The third case still stands though. It can be reproduced with this xUnit test (which fails).

[Fact]
public void Scenario3__Columns_are_not_quoted()
{
    var query = new Query("users").Select("Name");

    var sql = new PostgresCompiler().Compile(query).Sql;

    Assert.Equal("SELECT Name FROM users", sql);
}

I should have made the repro before reporting the issue, so sorry to bother you needlessly with scenario 1 and 2.

ahmad-moussawi commented 1 year ago

no worries thanks for getting back, so I think we can close this ticket now, feel free to reopen in case you find something else.