-
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
NewObjectExpression: Allows use of nested new operators #6709
NewObjectExpression: Allows use of nested new operators #6709
Conversation
fa93d0f
to
310b071
Compare
foreach ($rowData['newObjects'] as $objIndex => $newObject) { | ||
$class = $newObject['class']; | ||
$args = $newObject['args']; | ||
ksort($args); |
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.
Needs test case (probably when new object is given as not last argument)
@@ -324,6 +325,30 @@ protected function gatherRowData(array $data, array &$id, array &$nonemptyCompon | |||
} | |||
} | |||
|
|||
foreach ($this->_rsm->nestedNewObjectArguments as $objIndex => ['ownerIndex' => $ownerIndex, 'argIndex' => $argIndex]) { |
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.
Looks like scrutinizer doesn't know how to deal with destructuring.
@fesor looks like this patch is going in a nice way, however I wonder that we should rather do this on v3 instead. Not sure about what @guilhermeblanco and @Ocramius think about this, but I believe it's about time to do a feature freeze on 2.x. |
This will unlikely land in 2.x due to the current pressure to get 2.6 out
and start with 3.x overall.
I'd say we leave it as-is and ask for a rebase once `develop` is merged.
…On 29 Oct 2017 23:25, "Luís Cobucci" ***@***.***> wrote:
@fesor <https://github.com/fesor> looks like this patch is going in a
nice way, however I wonder that we should rather do this on v3 instead. Not
sure about what @guilhermeblanco <https://github.com/guilhermeblanco> and
@Ocramius <https://github.com/ocramius> think about this, but I believe
it's about time to do a feature freeze on 2.x.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6709 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJakOj_7hEEihbrvsxdX8SKYpEs6UGSks5sxPtugaJpZM4PceAv>
.
|
310b071
to
93bb1a5
Compare
93bb1a5
to
f7c4643
Compare
@Ocramius rebased on master. |
Very nice! Does it handle collections ? eg if User has multiple addresses and constructor param is an array in UserDTO |
@tkleinhakisa you mean handling *-to-many relations? Currently no... need to check how many changes in hydrator is required to make it possible. I think that whole hydration system should be refactored so this and many other use cases could be possible without huge overhead, like the one in #2553 for example. But in order to make things both maintainable and performant, this would require some kind of inlining functionality and code generation... |
I'd probably suggest to postpone further work here for now if it requires further redesign. 2.x branch is in feature-freeze and for 3.0 we've been discussing a full DQL parser rewrite. No roadmap for that yet, but I've already been prepping a switch to Hoa\Compiler. |
What are the chances of this making it into v3? |
It's definitely something I'd like to pursue since I believe DTOs have high potential, but likely not before rewriting the parser itself, for that see #7017. |
I'm closing this PR because it hasn't seen progress for several years. Please open a new one if you want to pick up the work one day. |
Hoa packages are abandoned. Could this be reconsidered? |
|
Covers #4687, #6573
This PR adds ability to use aggregation of DTOs as query result:
Use cases: