From 34424c390cf909a437c80558088f7ef7962da7ed Mon Sep 17 00:00:00 2001 From: demiliani Date: Thu, 1 Oct 2026 11:05:00 +0200 Subject: [PATCH] Preserve business filters when recommending Record.Get --- evaluation/review-fixtures.json | 1 + ...ad-of-findfirst-on-full-primary-key.bad.al | 9 ++++++++ ...d-of-findfirst-on-full-primary-key.good.al | 21 +++++++++++++++++++ ...nstead-of-findfirst-on-full-primary-key.md | 18 +++++++++++----- 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index 5f0e50a..b8aa3ed 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -58,6 +58,7 @@ "performance": { "articles": [ "use-isempty-for-existence-check", + "use-get-instead-of-findfirst-on-full-primary-key", "job-queue-category-code-serializes-conflicting-jobs", "job-queue-external-effects-must-be-idempotent", "job-queue-handlers-must-not-require-ui", diff --git a/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.bad.al b/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.bad.al index 34f4ec3..4b48ef9 100644 --- a/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.bad.al +++ b/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.bad.al @@ -8,4 +8,13 @@ codeunit 50211 "Perf Sample GetByPK Bad" if Customer.FindFirst() then Message(Customer.Name); end; + + procedure ShowUnblockedName(CustomerNo: Code[20]) + var + Customer: Record Customer; + begin + Customer.SetRange(Blocked, Customer.Blocked::" "); + if Customer.Get(CustomerNo) then + Message(Customer.Name); + end; } diff --git a/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.good.al b/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.good.al index d625150..3cdf03f 100644 --- a/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.good.al +++ b/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.good.al @@ -7,4 +7,25 @@ codeunit 50210 "Perf Sample GetByPK Good" if Customer.Get(CustomerNo) then Message(Customer.Name); end; + + procedure ShowUnblockedName(CustomerNo: Code[20]) + var + Customer: Record Customer; + begin + Customer.SetRange("No.", CustomerNo); + Customer.SetRange(Blocked, Customer.Blocked::" "); + if Customer.FindFirst() then + Message(Customer.Name); + end; + + procedure ShowUnblockedNameByKey(CustomerNo: Code[20]) + var + Customer: Record Customer; + begin + if not Customer.Get(CustomerNo) then + exit; + if Customer.Blocked <> Customer.Blocked::" " then + exit; + Message(Customer.Name); + end; } diff --git a/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md b/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md index e74614b..154e01d 100644 --- a/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md +++ b/microsoft/knowledge/performance/use-get-instead-of-findfirst-on-full-primary-key.md @@ -1,26 +1,34 @@ --- bc-version: [all] domain: performance -keywords: [get, findfirst, primary-key, setrange, lookup] +keywords: [get, findfirst, primary-key, setrange, lookup, filters, blocked] technologies: [al] countries: [w1] application-area: [all] --- -# Use Get when the full primary key is known; FindFirst is the wrong tool +# Use Get for primary-key lookups without losing filter conditions ## Description -`Get(...)` is the direct primary-key lookup. `FindFirst()` walks an index — even when narrowed by `SetRange` on every primary-key field. The upstream review guidance treats `Customer.SetRange("No.", CustomerNo); if Customer.FindFirst() then ...` as a bad pattern and `if Customer.Get(CustomerNo) then ...` as the correction. The two reach the same record; only `Get` expresses the lookup as a primary-key seek. +`Get(...)` retrieves a record by primary key, but ignores normal record filters. Replacing a `FindFirst()` filtered only by the full primary key with `Get` expresses the lookup directly. The same replacement is not equivalent when additional filters enforce business conditions, such as requiring an unblocked customer. Security filters are a separate mechanism: their effect on `Get` depends on Security Filter Mode. ## Best Practice -When all primary-key fields are available at the call site, call `Get` (or `GetBySystemId`) with them. Reserve `FindFirst` for cases where the filter is on something other than the full primary key — a unique secondary field, a partial composite key, a sort that the caller cares about. +When all primary-key fields are available and no additional normal filter constrains the result, call `Get` with them. Reserve `FindFirst` for filtered searches, including partial keys, secondary fields, or additional business conditions. + +Before recommending a replacement, inspect the effective filters at the call site, including filters set by callers or helpers. If additional conditions matter, retain the filtered `FindFirst` or explicitly enforce equivalent conditions after a successful `Get`, before using the record. Do not flag `FindFirst` merely because all primary-key fields are filtered when a non-key filter must also hold. A `SetRange` before `Get` does not enforce that condition. See sample: [`use-get-instead-of-findfirst-on-full-primary-key.good.al`](use-get-instead-of-findfirst-on-full-primary-key.good.al). ## Anti Pattern -Composing `SetRange` calls that exactly cover the primary key and then calling `FindFirst`. The result is correct but the call site reads as "search the table" rather than "look up by key", which obscures both the intent and the access pattern from later reviewers. +Composing `SetRange` calls that cover only the full primary key and then calling `FindFirst` obscures a direct key lookup. Do not extend this finding to a lookup with additional business filters unless the proposed correction preserves them. + +Replacing a filtered lookup with `Get` while assuming a normal filter still excludes records is a correctness defect: a blocked or otherwise ineligible record can pass the lookup. Recommending that replacement without preserving the condition is also an incorrect review finding. See sample: [`use-get-instead-of-findfirst-on-full-primary-key.bad.al`](use-get-instead-of-findfirst-on-full-primary-key.bad.al). + +## References + +- [Record.Get remarks: primary-key lookup and filter semantics](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-get-method).