a9f3c50863
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.3 KiB
| bc-version | domain | keywords | technologies | countries | application-area | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
upgrade |
|
|
|
|
Guard every database read in upgrade code with if
Description
Inside an upgrade codeunit (or any procedure transitively invoked from OnUpgradePerCompany / OnUpgradePerDatabase), an unguarded Record.Get, Record.FindSet, or Record.FindLast raises a runtime error when the row or set is missing. In upgrade context that error aborts the entire upgrade for the company or database — a far worse outcome than the missing data itself. Records the upgrade reasons about may legitimately not exist on every customer's tenant.
Best Practice
Wrap every read in an if. if Item.Get(No) then ..., if Customer.FindSet() then;, if not Vendor.FindLast() then exit;. The empty-then form if Customer.FindSet() then; is the idiomatic way to attempt a read whose only purpose is to position a record, while swallowing the "not found" case.
See sample: guard-database-reads.good.al.
Anti Pattern
Calling Item.Get(), Customer.FindSet(), or Vendor.FindLast() bare in upgrade code. The first tenant whose data does not match the upgrade's assumptions will fail to upgrade.
See sample: guard-database-reads.bad.al.