The previous LLM-generated knowledge files contained factual
hallucinations. The most visible was the claim that `FindFirst` /
`FindLast` "forces a full-table scan" on an unfiltered record - it does
not; those APIs return a single row via the current key.
Other inaccuracies the audit found and fixed:
* `FindSet(true)` was described as "taking a LockTable". The correct
upstream phrasing is that `FindSet(true)` sets
`ReadIsolation::UpdLock` on the read. UpdLock and LockTable are
related but distinct mechanisms.
* The list of production-scale tables had been invented beyond the
upstream source (e.g. "Detailed Cust. Ledg. Entry") without a
citation. The regenerated list matches the ten tables upstream lists
with their P95 row counts.
* `SetLoadFields` guidance had been augmented with an extra mechanism
claim ("the database resolves the filter using the index without
hydrating the value") not present in upstream.
Approach: full regeneration of `microsoft/knowledge/` from the six
upstream BCApps Code Review instruction files, with Microsoft Learn /
the AL language reference as a secondary source. Every claim in every
regenerated file is anchored to a verbatim upstream quote (or a Learn
URL); the audit trail lives in artifacts/trace-<domain>.json on the
session workspace.
The PR #11 transaction/error-handling cluster is preserved verbatim:
* performance/understand-implicit-transaction-boundary.md
* performance/codeunit-run-as-atomic-sub-operation.{md,good.al,bad.al}
* performance/codeunit-run-requires-prior-commit-inside-transaction.{md,good.al,bad.al}
* performance/use-tryfunction-for-error-catching-not-rollback.{md,good.al,bad.al}
* performance/avoid-commit-inside-loops.{md,good.al,bad.al}
* security/commitbehavior-attribute-scopes-explicit-commits.{md,good.al,bad.al}
* testing/transactionmodel-attribute-governs-test-transactions.{md,good.al,bad.al}
These articles already cite Microsoft Learn and were carefully
cross-referenced; the regeneration skips their topics rather than
duplicating them.
File counts after regeneration:
performance 35 .md (5 preserved + 30 new)
privacy 17 .md
security 18 .md (1 preserved + 17 new)
style 33 .md
testing 1 .md (preserved)
ui 19 .md
upgrade 18 .md
Total 141 atomic knowledge files, each strictly one rule. All pass
.github/scripts/validate_frontmatter.py with 0 errors and 0 warnings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1.7 KiB
| bc-version | domain | keywords | technologies | countries | application-area | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
performance |
|
|
|
|
Pass false to Insert/Modify/Delete when the table triggers do not need to fire
Description
Insert(true), Modify(true), and Delete(true) run the table's OnInsert/OnModify/OnDelete trigger; the (false) form skips it. Per the upstream guidance, the trigger form should be used "only when needed" — every row whose write fires a trigger pays that cost, even when the trigger has nothing useful to add for the current call site. For tight bulk write paths the difference compounds linearly with row count.
Best Practice
Reach for the (false) form when the calling code already enforces the invariants the trigger would, or when the trigger is empty for the current table/extension. Use (true) when the trigger does work the caller depends on (number-series allocation, validation, cascading writes). Decide per call, not by code style: a default of "always true" makes bulk writes pay for triggers they did not need, and a default of "always false" silently skips validation the trigger was put there to enforce.
See sample: pass-false-to-insert-when-trigger-not-needed.good.al.
Anti Pattern
Looping over thousands of rows and calling Modify(true) on each, when the table's OnModify trigger does nothing relevant for the operation. The trigger cost is paid per row; the user-visible behavior is identical to the (false) form. The mirror is using (false) for an operation that depends on trigger-side defaulting and silently producing rows that fail downstream validation.