nullism / bqb

BQB is a lightweight and easy to use query builder that works with sqlite, mysql, mariadb, postgres, and others.
MIT License
154 stars 13 forks source link

Error in subquery gets lost #22

Closed HenrikPoulsen closed 1 year ago

HenrikPoulsen commented 1 year ago

I am really enjoying this project.

One issue I noticed very early is that if there's an error in a separate subquery it does not propagate when added to an outer query.

See this test I wrote to illustrate the issue

func TestErrorSubquery(t *testing.T) {
    // One more ? than we are passing args for which is wrong and should fail
    subQuery := New("SELECT id FROM table2 WHERE id = ? and value = ?", 1)

    // It gives an error when the query is directly turned to sql which is good
    _, _, err := subQuery.ToSql()
    if err == nil || !strings.Contains(err.Error(), "extra ? in text") {
        t.Errorf("expected error for subquery")
    }

    // But if you pass it into another query instead there is no error and the sql it spits out will be
    // "SELECT * FROM table WHERE id IN ()" which is not what I would expect
    q := New("SELECT * FROM table WHERE id IN (?)", subQuery)
    _, _, err = q.ToSql()
    if err == nil {
                // It fails here
        t.Errorf("expected error for subquery")
    }
}

Is it possible to get it to check the sub queries for errors and return those instead of ignoring them?

nullism commented 1 year ago

Hey! This is a great idea. Thanks for the feedback :)

Do you mind sharing what version you're using? I had thought we were propagating errors since v1.4.0+, but this may not be the case.

HenrikPoulsen commented 1 year ago

Yeah I noticed this on v1.6.1 and reproduced it on latest main in this repo with the test. I created #23 with a proposed fix. It just seems like something was missed, because the other cases did handle the errors