-
Notifications
You must be signed in to change notification settings - Fork 549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support $n arguments in SQL fragments other than SQL()
#1277
Comments
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. |
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.
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. |
Yeah I've been feeling this fragility of the code when trying to solve these issues myself: (incomplete): 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 Anyways, I opened #1278 for the second issue. |
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. |
Wow, that looks very promising! Already comparing this to this it looks much more trustworthy. Would you consider supporting |
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 |
Love it, thanks for the guidance! |
Hey, I have a query like so:
And I get
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.The text was updated successfully, but these errors were encountered: