-
Notifications
You must be signed in to change notification settings - Fork 532
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
MySQL Syntax Error in ActiveRelationResource (0.10.x) #1369
Comments
Link to GH Actions for my fork, which include the failing tests and MySQL syntax error message: |
Version 0.10.5 generated invalid MySQL queries cerebris/jsonapi-resources#1369 Pinning to earlier version until this is fixed.
(using 0.10.7 release)
but then in
which results in the new sql
specifically this becomes a problem I don't fully understand what is going on at a low level, but it seems like something extremely simple like highlights that active record is sanitizing the wrong part of the field path.
Anyway, this might all be gibberish and I am looking at everything wrong but if anyone has some thoughts I would be happy to continue hacking away as we are dealing with a crazy legacy database schema so if there is an edge case, I bet we'll see it. |
I too was affected by this issue (i'm using MySQL) and after some digging can confirm it's a regression with version 0.10.7. This release uses a newly added
The problem is this quoting is not relying on the database connection adapter. With limited time I couldn't figure out how to obtain the resource class in question so my fix was re-defining the quote method created from this library in my base resource:
Of course this assumes a single underlying database connection 🤷 |
Using the previous method to generate the quote string could lead to incorrect sql statement where double quotes were used for select statements in MySql. By using the `quote_table_name` function from Rails, the correct quoting mechnism will be automatically used for the specific database. Fixes cerebris#1369
|
I've "written" a failing test , linked here: #1395 (comment) ; can we turn on MySQL in the test matrix? |
it looks like mysql being turned on in .11 release: master...v0-11-dev we can close this once that gets merged down I believe |
Implementing fix from issue cerebris#1369. cerebris#1395
This issue is a (choose one):
Checklist before submitting:
Description
Choose one section below and delete the other:
Bug reports:
Version
0.10.5
This is related to #1310, but is not a multi-tenancy issue. This a MySQL syntax issue that I believe it created by the
ActiveRelationResource#quote
method which is uses explicit double quotes for quoting table names inActiveRelationResource#concat_table_field
andActiveRelationResource#alias_table_field
.By adding MySQL to the CI workflow testing matrix, this issue is easily reproduced in CI testing. I will create a PR for adding MySQL to the testing matrix.
The text was updated successfully, but these errors were encountered: