SFDO-Community/declarative-lookup-rollup-summaries

Known ordering for RollupCalculateJob records

rogeramitchell opened this issue ยท 10 comments

Hey there, I'm a huge fan of the tool and recommend it to anyone that needs rollups (virtually everyone).

I think there might be a feature missing to ensure there is a known order of records when using the "Calculate" or "Schedule Calculate" feature. It appears that RollupCalculateJob is getting the query locator from RollupService.masterRecordsAsQueryLocator, though I do not see where this is implementing an ORDER BY clause.

If the user were to start a calculation for a rollup, it does not seem that the user should need to wait until that calculation finishes to begin the next rollup (provided the same parent object exists).

If the query returned from RollupService.masterRecordsAsQueryLocator were to use an ORDER BY clause as part of the query string, the order would be predictable, and thus the rollups could be staggered to start by a few minutes.

  1. Is the statement above correct (re: predictable order)?
  2. If so, would you like me to submit a PR with the proposed changes?

I do not think the below solves for this, but figured I'd include it (in the event that it does)
https://releasenotes.docs.salesforce.com/en-us/summer15/release-notes/rn_apex_maps_and_sets_iteration_order.htm

Another resource suggests that there is no guarantee of query result order without the inclusion of an ORDER BY clause in the query string.

https://salesforce.stackexchange.com/questions/7667/soql-result-ordering-in-the-absence-of-an-order-by-clause

Great idea, thanks for sharing! ๐Ÿ‘ Marked as enhancement.

My pleasure, thank you for your work on the project @afawcett! Do you want me to submit a PR with the change?

@renatoliveira Yes please! ๐Ÿ‘

@afawcett I think you meant that to @rogeramitchell ? ๐Ÿ˜†

;-) I did, thanks @renatoliveira ๐Ÿ‘

@afawcett submitted #536 as PR, looks like CI failed due to username being blank for the build. Not sure whether this is something I can resolve. Mind taking a look?

Yeah thats an issue with the way the CI works in pull requests, don't worry. I'll have a look at your PR now, thanks for the submission! ๐Ÿ‘

This looks great, just merged. I will get it out in the next release. I will look to carve out some person time for some popular fixes/enhancements and include your work in this. Thanks again! ๐Ÿ‘

Fixed in v2.10