Bugfix/column names from subquery #1714
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trying to get ModelCriteria::addSelectQuery(): working.
Found several issues:
FROM
(addressed here)ModelCriteria::select()
are not resolved` (addressed here)This addresses subquery column name replacement in outer query.
Problem example:
leads to query
The error is quite simple: When replacing the column literal from a subquery, the column map was also set, which causes the column to be treated as a column belonging to the outer table. Not setting the column map resolves the issue.
Note that the whole column name replacement is very smelly. Look at what happens with the
currentAlias
class variable if you want to get a whiff. I initially cleaned this up, but realistically, a large commit like this is not going to get reviewed properly, so boiling it down seems like the best option. During refactoring, I found that tests for name replacement were scattered across three files. That cleanup I kept, because it is probably helpful for others. It is in its own commit, should you want to review it separately.Oh, I have cleaned up two other minor, yet pretty stunning semantic mistakes that I found:
$this
, when$this
generally does not have them. Pretty sad that Pstan was just silenced, when it correctly complained about it.I'll highlight them in the review, so it does not confuse anyone.
Btw. I think theese mistakes highlight a larger issue with the Criteria classes, where it is not clear from their name what they do (what`s a "Critera", particularly in contrast to a "Criterion", and why does it create sql code?), and where extension hierarchy was abused to decompose the classes, leading to arbitrary logical splits between them. This has apparently and understandably left people confused what goes where, including me.