knowledge(performance): align SetLoadFields placement with AL Guidelines - #130
Open
Jesper Schulz-Wedde (JesperSchulz) wants to merge 1 commit into
Open
Conversation
…nes (#120) The AL Guidelines mark `SetLoadFields` placed before `SetRange`/`SetFilter` as bad code and recommend filters first, while the BCQuality samples used the opposite order — contradictory guidance across two Microsoft repos. Per Learn (`Record.SetLoadFields`), "fields that are filtered upon are always loaded", so the two orders produce an identical projection. The upstream rule is a readability convention: keep `SetLoadFields` adjacent to the read it governs. - Reorder filters ahead of `SetLoadFields` in the six affected AL samples. - State the placement convention in the Best Practice section. - Record in Description that order does not change the projection, and that only a fieldless `SetLoadFields()` or a later overwriting call does. - Add an Anti Pattern note so review agents treat the reverse order as a readability observation, never a performance defect. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Jesper Schulz-Wedde (JesperSchulz)
requested review from
Bardur Knudsen (BardurKnudsen) and
Bugsy (pchriste-microsoft-com)
as code owners
August 17, 2026 11:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves the contradiction reported in #120.
The question
The AL Guidelines mark
SetLoadFieldsplaced beforeSetRange/SetFilteras bad code; BCQuality's samples used exactly that order.The answer
The order is a readability convention, not a behavioural one. Per
Record.SetLoadFieldson Learn:So both orders materialize an identical projection.
SetRange/SetFilternever resets the load-field selection — only a fieldlessSetLoadFields()(which resets to all readable normal fields) or a laterSetLoadFields(...)overwriting an earlier one changes what is loaded.The upstream rule is about keeping
SetLoadFieldsadjacent to the read it governs — the same rationale as the AL Guidelines' case-statement example, which BCQuality already mirrors inload-common-fields-before-branching-on-case.md. That page is taggedReadability, notPerformance.Changes
BCQuality now follows the AL Guidelines order, and states explicitly that the order is not technically relevant so the two repos cannot be read as contradicting each other.
SetLoadFieldsin the six affected AL samples:use-setloadfields-for-partial-records.{good,bad}.alload-only-primary-key-fields-for-reference-work.good.alavoid-cloning-records-before-modify-delete-in-loops.{good,bad}.aluse-setautocalcfields-for-per-row-flowfields.{good,bad}.aluse-setloadfields-for-partial-records.md:SetRange/SetFilter.SetLoadFieldsdoes.The Anti Pattern placement is deliberate: per
skills/read.md, only## Best Practiceand## Anti Patternare normative, so a false-positive suppression has to live there to be honoured by consumers.Validation
All three CI validators pass locally:
validate_frontmatter.py— 0 errors, 0 warningsTest-KnowledgeIndex.ps1— 255 articles, deterministic, full coverageTest-ReviewFixtures.ps1— 32 cases across 16 leaf domainsCloses #120