mirror of
https://github.com/doctrine/orm.git
synced 2026-03-23 22:42:18 +01:00
DDC-1047: Combining explicit join syntax with multiple from-clause argument-list entries will generate broken SQL on MySQL #1304
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @doctrinebot on GitHub (Feb 26, 2011).
Originally assigned to: @beberlei on GitHub.
Jira issue originally created by user dalvarez:
When using explicit join syntax, MySQL requires the from-clause argument-list to have either only one entry, or, if it contains multiple entries, it requires that the complete argument-list be surrounded by parantheses. Currently, Doctrine 2 does not respect this requirement when compiling DQL statements, and produces broken SQL in some situations.
Consider the following example of a legitimate DQL query.
This will be compiled down to:
This SQL query can't be executed, because MySQL will complain. The error message is "SQLSTATE[42S22]: Column not found: 1054 Unknown column 'v0_.vendorListXML_dbID' in 'on clause".
The resulting error gives no sensible error message, and is a pain to debug, because obviously, the column mentioned does exist. The real cause of the error is that MySQL will cease to correctly interpret the aliases given, as long as parantheses not are used. I acknowledge that the MySQL behavior is downright stupid. This might well be considered a bug in MySQL, but unfortunately, the parantheses are documented in their syntax docs (http://dev.mysql.com/doc/refman/5.0/en/join.html), though I could not find any official statement regarding the use.
Fortunately it is easy to patch Doctrine to support it.
Now, let's add parantheses around the from clause argument list of the generated SQL statement:
@doctrinebot commented on GitHub (Feb 26, 2011):
Comment created by dalvarez:
Here's a patched Doctrine\ORM\Query\SqlWalker that implements the additional parantheses around the from-clause argument-list.
Consider this prototype quality, for testing and verification purposes. I am sure it could be implemented more cleanly.
I have not yet taken the time to get the big picture of where this can be sustainably implemented in Doctrine 2 (specially being probably MySQL-specific), but I needed this to work quickly, so this is a first shot at it.
@doctrinebot commented on GitHub (Mar 1, 2011):
Comment created by @beberlei:
Fixed formating
@doctrinebot commented on GitHub (Mar 1, 2011):
Comment created by dalvarez:
It actually seems to be an SQL 2003 compliance thing.
http://bugs.mysql.com/bug.php?id=13551
See post from 8 Oct 2005 12:12.
@doctrinebot commented on GitHub (Mar 1, 2011):
Comment created by dalvarez:
It's becoming even worse since the problem is actually more general. Consider this query:
I do not want to bother you with the details of this data model. Suffices to say that WPOMOrder inherits from VersionedDataObject.
This will be compiled down to:
... which introduces ambiguity, because the inheritance is inserted before the second FROM-clause entry is added. Which means, the "first shot" fix I attached will not work for this case.
The solution would have to make sure that this kind of ambiguity on SQL 2003-compliant systems is generally eliminated from the generated queries, by proper use of parantheses in all situations.
@doctrinebot commented on GitHub (Mar 1, 2011):
Comment created by dalvarez:
Note that the second example runs on the patched SQLWalker, which still does not cover the inheritance declaration properly.
Without the patch, it would already produce wrong SQL in the first example, of course.
@doctrinebot commented on GitHub (Mar 1, 2011):
Comment created by dalvarez:
The query shown above needlessly queries the entity XMLDocument, which, fortunately, is no longer needed in this use case. This being fixed, to me it is no blocker bug, but there are cases where inheritance will actually be used in conjunction with multiple from-clause entries, and I am pretty sure such scenarios will arise, so I leave this query here in the comments to illustrate the problem.
@doctrinebot commented on GitHub (Mar 19, 2011):
Comment created by dalvarez:
Increased priority to critical.
Mind you, this bug causes Doctrine to regularly generate broken queries on MySQL. Without my local patch, it would be unusable for my use case.
I believe a fix for this would be crucial. Doctrine absolutely needs to generate SQL 2003 compatible FROM clauses.
@doctrinebot commented on GitHub (Mar 19, 2011):
Comment created by dalvarez:
Attached new patched SQLWalker, now extended to add parantheses around FROM clauses within subqueries. Again, this is prototype quality, but it apparently fixes the problem.
@doctrinebot commented on GitHub (Mar 19, 2011):
Comment created by dalvarez:
Increased priority to blocker.
Now I have a query that relies on multiple FROM-clause argument-list entries, combined with JOIN expressions, that relies on an inheritance join as well. This is the type of query I described in my comment from 01/Mar/11 05:56 PM. In such a case, Doctrine 2 will generate invalid code leading to the following MySQL Server Error:
SQLSTATE[42S22]: Column not found: 1054 Unknown column [...] in 'on clause.
The column mentioned exists, though, MySQL Server just can not properly understand the aliases given, because the generated query is ambiguous, and MySQL Server is smart enough to complain about it.
A quick fix would be to generally move inheritance join expressions past the regular FROM-clause entries, and use proper parantheses around the list of regular (i . e. non-join table-reference) FROM-clause argument-list entries.
Please fix this ASAP. This is fundamental.
@doctrinebot commented on GitHub (Mar 20, 2011):
Comment created by @beberlei:
This fails on PostgreSQL aswell, not on SQLite however. I cant run Oracle currently as my machine with that is broken.
The problem here is twofold:
Support for SQL 2003 is not about magic parenthis work magically for use-case 2, it should just be possible to do:
@doctrinebot commented on GitHub (Mar 20, 2011):
Comment created by @beberlei:
It is already possible to do this:
@doctrinebot commented on GitHub (Mar 20, 2011):
Comment created by @beberlei:
Just tested a bit, it is not even possible to do automatic parenthesis around inheritance hierachies as the aliases go out of scope for the WHERE clause, in PostgreSQL there are other scoping errors with your patch. This all has to be explicit, which makes a fix for this more complicated.
@doctrinebot commented on GitHub (Mar 20, 2011):
Comment created by @beberlei:
Wrapped SQL Codes new, very annoying that they dont overflow (Jira annoyances)
@doctrinebot commented on GitHub (Mar 20, 2011):
Comment created by @beberlei:
Changed back to critical, this is not a blocker, you can always use Native SQL to get around limitations of DQL. Its not pretty, but its not a blocker to usage, given that you can rearrange the FROM and JOIN clauses one of your issues should be solved by existing code, only the inheritance persists (not sure if that not goes away also).
@doctrinebot commented on GitHub (Mar 20, 2011):
Comment created by dalvarez:
Thanks for taking care.
This way you should never have to deal with blockers, just by providing a way to directly use PDO, which you always should be able to use directly anyway. ;-)
My query is quite complex because I had to replicate certain sections to compensate for the lack of support for CASE expressions. So it would be a pain to translate it to SQL, not even talking of maintaining it. But yes, technically nothing is really a blocker, though it is fair to say that workarounds can be impractical.
I will have a try at the solution you proposed in your comment from 20/Mar/11 05:43 AM. The basic parantheses problem is solved by the local SQLWalker patch. But it does not cover the inheritance join, which gets generated right amid the regular FROM-clause entries, instead of at the end with the other JOIN expressions, thus generating broken SQL.
So a sustainable fix would have to fix the inheritance join problem. My guess is it would help to handle the JOIN expression of the inheritance join the same way as other JOIN expressions, and render the statements for both after the regular FROM-clause entries.
@doctrinebot commented on GitHub (Mar 20, 2011):
Comment created by @beberlei:
The complexity of this query does not stem from the select clause, but from the query and matching logic. That is not much more complicated in plain sql than in DQL. The complexity of using NativeSQL over DQL is imho complex fetch join or inheritance scenarios.
I consider something a blocker only if its a bug that has no temporary workaround.
Your SQL Walker patch breaks the testsuite (not the sql generation queries, but real functional tests), so i cannot apply it.
Inheritance i think cannot just be put to the end of all from clauses, i'd rather have explicit mechanisms such as:
I also have to check how JPQL or HQL solves this issue.
@doctrinebot commented on GitHub (Mar 20, 2011):
Comment created by dalvarez:
Hey, you have not seen the query yet... ;-)
But yes, I agree it would be possible, though annoying, to write any query in plain-SQL.
The "patch" is just a prototype, to illustrate the issue. For me it works. It could be, of course, that it breaks some other feature my application does not use.
As for the inheritance joins, I do not yet understand why they should be handled differently than any other joins. The RDBMS should be agnostic to the reason a join is done, it just knows the basic mechanism, which should be the same.
@doctrinebot commented on GitHub (Mar 21, 2011):
Comment created by dalvarez:
Changed affected Version to 2.0.2. Actually, 2.0.3 is affected too, but I cannot select it.
@doctrinebot commented on GitHub (Mar 21, 2011):
Comment created by dalvarez:
Same SQLWalker first-shot concept patch, now based on Version 2.0.3
@doctrinebot commented on GitHub (Mar 21, 2011):
Comment created by dalvarez:
Ben,
is it possible to just generate the code for the inheritance joins at the end of the FROM clause?
Combined with always adding parantheses around the list of regular FROM clause entries, I believe, this could very well solve the problem. And it could turn out to be just a little manageable change.
There would not be any need for explicitly adding parantheses then, from a user-programmer perspective.
@doctrinebot commented on GitHub (Mar 22, 2011):
Comment created by dalvarez:
I uploaded a new SQLWalker.
This is a first shot at handling inheritance joins just like regular joins (generating an expression after the non-join table references, rather than between them). On MySQL 5.0, the patched code seems to generate valid SQL for all the use cases of my application.
Could you please run the unit tests against it, to see if the changes break anything?
@doctrinebot commented on GitHub (Mar 24, 2011):
Comment created by dalvarez:
I just gave the patched SQLWalker a try on PostgreSQL 8.4, and it did not work. PostgreSQL would complain about syntax errors, as the parantheses are not expected. I will see later this day if PostgreSQL requires the additional parantheses at all. Maybe on PostgreSQL the queries work fine without the parantheses in the first place. Given that PostgreSQL complains about syntax errors when provided with the additional parantheses, this is to be expected, but I will let you know as soon as I know for sure.
@doctrinebot commented on GitHub (Mar 28, 2011):
Comment created by dalvarez:
Sorry for the late update. I had to fix a RDBMS-dependent UTF-8 issue in my application before being able to fully test the relevant queries with PostgreSQL
Now that I did, PostgreSQL seems to have the same problem like MySQL Server.
Take this DQL query for example:
On PostgreSQL 8.4.7, it will get compiled down to:
... which will not work, because v0 can not be referenced by the first JOIN expression.
Here is the log:
Adding parantheses around the non-join FROM clause argument-list entries the way it worked for MySQL 5.0 will not work here. PostgreSQL will complain about a syntax error:
So the bottom line is that the problem does exist on PostgreSQL as well, but the quick fix that apparently worked for MySQL 5.0 will not work here.
@doctrinebot commented on GitHub (Mar 28, 2011):
Comment created by dalvarez:
Note that PostgreSQL claims very specific compliance with the relevant packages of the SQL 2008 standard, while the MySQL documentation is, as usual, broadly inaccurate and amateurish about that topic. It does not matter here, if version 5.0 or 5.1 of MySQL is concerned.
http://www.postgresql.org/docs/8.4/static/features.html
http://dev.mysql.com/doc/refman/5.0/en/standards.html
When in doubt, I would suggest to simply aim for SQL 2008 compliance (as later versions of the standard supersede the older ones), so as to not risk optimizing for MySQL peculiarities rather than for general compatibility. This should make it possible to define a single level of compliance, independent of the specific RDBMS.
@doctrinebot commented on GitHub (Mar 28, 2011):
Comment created by dalvarez:
Removed long PostgreSQL log line...
@doctrinebot commented on GitHub (Apr 2, 2011):
Comment created by dalvarez:
Are you still with me?
@doctrinebot commented on GitHub (Apr 2, 2011):
Comment created by @beberlei:
I am just super busy right now. Doing my best..
@doctrinebot commented on GitHub (Apr 3, 2011):
Comment created by dalvarez:
OK!
@doctrinebot commented on GitHub (Apr 3, 2011):
Comment created by @beberlei:
Hm, just tried a little bit. What about:
@doctrinebot commented on GitHub (Apr 3, 2011):
Comment created by @beberlei:
I think this query will work for you without any changes to the SQLWalker:
@doctrinebot commented on GitHub (Apr 4, 2011):
Comment created by dalvarez:
Hey thanks, I'll try doing it that way.
Right now everything works fine with the local patch, at least on MySQL 5.0.
As for PostgreSQL, I will give it a run in a spare minute and let you know.
@doctrinebot commented on GitHub (Jul 31, 2011):
Comment created by @beberlei:
Any feedback on this issue?
@doctrinebot commented on GitHub (Aug 1, 2011):
Comment created by dalvarez:
I have not yet managed to change and test the queries of the application, because it is a major change affecting lots of complex queries that span literally several dozen pages each (we had to emulate the unsupported CASE statements with conditional logic and joins, which multiplied the lines of code in many queries).
I have postponed changing the queries until next week, when I will upgrade the application to Doctrine 2.1 after finishing some due tasks. Afterwards I will run the application code with changed queries according to the pattern described in your comment above, and let you know. I will give it a try on PostgreSQL then, too. Last time I checked, my local patch did not work on PostgreSQL. I hope the rewritten queries will.
@doctrinebot commented on GitHub (Aug 6, 2011):
Comment created by @beberlei:
I think your local patch is not necessary, you have to order the from and join clauses correctly (as required when writing SQL itself). This problem is "handled" by DQL by delegating the error handling to the SQL level as we don't evaluate semantical errors of SQL in DQL.
@doctrinebot commented on GitHub (Aug 7, 2011):
Comment created by dalvarez:
Sure, with the latest version it could work that way. I'll try, as I said.
The local SQLWalker patch was just a temporary measure. I hope it will become unnecessary after rewriting the queries the way you proposed.
@doctrinebot commented on GitHub (Sep 19, 2011):
Comment created by dalvarez:
Sorry for the delay. I am now upgrading to Doctrine 2.1 and rewriting the application's queries according to the way you explained. I will let you know about results within the next couple of days, after performing test runs based on the reworked queries.
@doctrinebot commented on GitHub (Sep 22, 2011):
Comment created by dalvarez:
I rewrote all queries to match the basic form you showed in your comment from April, 3rd.
Using the basic form
SELECT x
FROM drivingTable1 alias1
JOIN alias1.otherTable1 alias2,
drivingTable 2 alias3
JOIN alias3.otherTable2 alias4
...
the queries now work on MySQL Server without any local modifications of Doctrine 2.
I tested it with MySQL Server version 5.0.51a, and Doctrine version 2.1.1.
So far everything seems to be good.
I will try it on PostgreSQL, too, and let you know about that.
@doctrinebot commented on GitHub (Sep 29, 2011):
Comment created by dalvarez:
I tried a complex sample query on PostgreSQL using the same join syntax, and it worked like a charm.
I'll close the issue as fixed.
Thanks a lot.
@doctrinebot commented on GitHub (Sep 29, 2011):
Issue was closed with resolution "Fixed"
@doctrinebot commented on GitHub (Sep 29, 2011):
Comment created by dalvarez:
Issue closed.
I don't know about the DQL validation part, though. The fact that DQL can or could compile to something that is not valid SQL could be considered symptomatic of a bug by itself. Don't know about the state of that, maybe it's more robust by now.
@doctrinebot commented on GitHub (Dec 13, 2015):
Imported 4 attachments from Jira into https://gist.github.com/15e71d0756a6d338bccd