Support $n arguments in SQL fragments other than `SQL()`

[Javyre]

Hey, I have a query like so:

// generated predicate: "foo.bar = $1 AND foo.baz = $2"
predString, predArgs := MyPredicateGenerator(...)
models.Something(
    Where(predString, predArgs...), 
    // This turns into a where{} that refers to its own $1 arg.
    models.SomethingWhere.Kind.NIN([]string{models.SomethingKindEnumMatch}),
).Count(ctx, tx)

And I get

models: failed to count sources rows: expected 2 arguments, got 3

The problem is that the sql fragment in querymods are just concatenated together without renumbering the $n argument references. (While properly handling ?s).

This leads to confusing and inaccurate error messages.

It would be nice if the $n references in querymod sql fragments could be bumped just like ? so as to not leak this implementation detail of the querymod helpers to the user.

(In my case generating the $n references is much more straight-forward and maintainable than generating ?. I would like to specify the exact value I mean to refer to as opposed to depending on the position in the string and the arglist matching.)

What version of SQLBoiler are you using (sqlboiler --version)?

SQLBoiler v4.11.0 but applies to master AFAIK.

[Javyre]

Another uncovered bug here while looking at the code, there isn't any isolation between args coming from different fragments. In some places, fragments are concatenated together and then the concatenated string has the question marks replaced. So if one query mod uses one question mark too many and the next one uses one less, then we don't get the crucial error signalling that these two query mods are invalid and there most likely is an important bug present.

models.Something(
    // both these lines are malformed!
    qm.Where("? = ?", "abc"),
    qm.Where("1 = 1", "abc"),
).Count(ctx, tx)
// but the above code gives no errors and run's totally fine.

[stephenafamo]

The problem is that the sql fragment in querymods are just concatenated together without renumbering the $n argument references. (While properly handling ?s).

As far as I know, SQLBoiler does not work with dollar placeholders and you would have to use question marks, so the dollar placeholders are left entirely as-is.

So if one query mod uses one question mark too many and the next one uses one less, then we don't get the crucial error signalling that these two query mods are invalid and there most likely is an important bug present.

The way the query is built does not make this easy to catch, but this should probably be split into its own issue so that it can be tracked.

[Javyre]

Yeah I've been feeling this fragility of the code when trying to solve these issues myself: (incomplete): https://github.com/volatiletech/sqlboiler/compare/master...Optable:sqlboiler:bump-dollars

I feel like the querymod system being a flat list is getting in the way of more robust code (implementation would be much more robust and straight-forward with an ast.) and the convertInQuestionMarks feature is needless complexity (bug surface area) over something like https://github.com/Masterminds/squirrel/issues/104#issuecomment-331194825 .

Anyways, I opened https://github.com/volatiletech/sqlboiler/issues/1278 for the second issue.

[stephenafamo]

I feel like the querymod system being a flat list is getting in the way of more robust code

Not the querymod system itself, but the query object.

I have a different project heavily inspired by SQLBoiler which uses a new query builder that gets really dialect specific. It is called Bob. I also wrote a comparison vs SQLBoiler.

It wouldn't make sense for existing SQLBoiler users to switch, but I think Bob should be considered for new projects because it has a more "robust" foundation.

[Javyre]

@stephenafamo

I have a different project heavily inspired by SQLBoiler which uses a new query builder that gets really dialect specific. It is called Bob. I also wrote a comparison vs SQLBoiler.

Wow, that looks very promising! Already comparing this to this it looks much more trustworthy.

Would you consider supporting $n in your Raw sql fragments? Looks to me like a call to my bumpDollars above this line would potentially "just work"..

[stephenafamo]

Would you consider supporting $n in your Raw sql fragments? Looks to me like a call to my bumpDollars above this line would potentially "just work"..

I'll like to leave it out of Bob for simplicity, but it is fairly easy to create it as your own Expression type. As far as it implements the bob.Expression interface, you can use it when building the query.

[Javyre]

Love it, thanks for the guidance!