-
Notifications
You must be signed in to change notification settings - Fork 238
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
Enhance Order by to allow multiple fields and specify ASC/DESC #216
Comments
Nope your not overlooking a means to do, i've mark as an enhancement. |
Great, thanks! I started the work on this one and have the LRE changes already made and its working as expected. In my current approach I leveraged fflib_QueryFactory.Ordering which internally leverages fflib_QueryFactor.QueryField since it contains the functionality I need for this enhancement. The alternative is to replicate that same functionality directly within LREngine.cls. The only reason I can think of to replicate directly inside of LREngine.cls and not use the fflib classes is to abstract the enhancement so that those changes could be contributed directly to LRE project. Preference on which way to go? |
Please enhance LREngine if you can please. |
Note that this will complicate the situation where you can merge rollups into a single LREngine rollup call if the order by's are different. So makes things less efficient. This is somewhat unavoidable if the user wants this functionality. |
Thanks for the direction, I'll work within LREngine for the enhancement rather than leverage the fflib classes. Regarding merging rollups, I actually thought of this when I was going through this as I didn't see where merging "identical" rollups based on where clause and order by was currently being done. I'll take another look through the code and make sure to account for this when implementing the enhancement. Adding the additional orders by's shouldn't result in changing approach (need to look at the code first), but could result in not being able to merge if the user chooses different orders by's. As you say, unavoidable if the order by ultimately needs to be different. |
Cool and yeah the code is here. |
Thanks! 👍 |
…SC/DESC and NULLS FIRST/LAST
…d name case-(in)sensitivity
…lds and move to context Issue SFDO-Community#216 - Add support for multiple order by fields and optional ASC/DESC and NULLS FIRST/LAST Issue SFDO-Community#239 - Move order by clause to LREngine Context
Fixed in v1.24. |
It appears that there is currently a restriction to only support a single Order By field on rollups. In a quick review of the code, this might be a limitation at the LRE level. Would like to see support for allowing multiple order by fields and a way to indicate ASC/DESC sorting.
Use case is related to a concat rollup that is first based on a date and then by quantity. There could be multiples that have the same date so going to a 2nd and even 3rd level for ensuring consistent sort order is required.
Am I overlooking a way to do this or would this need to be an enhancement?
The text was updated successfully, but these errors were encountered: