-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Correct hydration of resultsets coming from DQL queries where the root of the selection is aliased and also the root of an inheritance #6367
Conversation
The main reason for test is to check that the Undefined Property error does not occur, and if you run the test without the fix, it will error out. It does however also make some assertions about the format of the result which may be questionable. The resultset only has one row, yet the hydrated result has two. My assumption was that this is intentional as this change did not break any tests, but it does seem odd to me. |
Might be worth running the tests again, only one job failed and it had just stalled before even getting to my code. I have no idea why my change would cause only PHP7.1/MariaDB to halt. |
@SirWaddles restarted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest LGTM, what do you think @Ocramius?
} | ||
|
||
/** | ||
* SELECT a as base, b, c, d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@group GH6362
missing
90bc02d
to
0a10f34
Compare
@SirWaddles I've rebased your branch to sync with the latest updates and group the commits. Patch LGTM but I'd love to have @guilhermeblanco's and/or @Ocramius' opinion just to double check. Thanks for the contribution 😉 |
Quite easy to backport for |
Good to go! Will also backport 👍 Thanks @SirWaddles, pristine work. |
Hah, I backported to |
Should be corrected in d2c805b (see https://travis-ci.org/doctrine/doctrine2/builds/227851219) |
Resolves Issue #6362
Added a test case for reproducability.