silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
720 stars 823 forks source link

ENH Enable allowing collisions for field statements #10957

Closed GuySartorelli closed 11 months ago

GuySartorelli commented 11 months ago

This one's a bit abstract as its own PR, but it makes sense in conjunction with the unit tests being added to https://github.com/silverstripe/silverstripe-framework/pull/10943 - specifically the "recursive CTE with extrapolated data" test scenario in DataQueryTest. That test (and indeed that entire query scenario) is not possible without allowing this, or something like this. Note that test uses an old name for the method - I'll update that once this and all other related PRs are merged.

It would probably be better to instead have a "coallesce" method, but the problem with that is we don't have a clean way to define the order of coalescence because a lot of fields aren't added until the query is finalised, so that method would be highly un-intuitive unless we made some breaking changes.

Issue

GuySartorelli commented 11 months ago

Yes, that does need to be done, and it will be once all of the dependant PRs are merged. Note that the test being referenced there is the test in https://github.com/silverstripe/silverstripe-framework/pull/10943 - not any tests in this PR.