mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 06:36:55 +01:00
Merge pull request #135 from microsoft/bugs/647560FixSetCurrentKeyHallucination
Correct severe misconception about SetCurrentKey in the knowledge base
This commit is contained in:
commit
9203e3ce2b
6 changed files with 74 additions and 40 deletions
|
|
@ -15,7 +15,7 @@ A `SetRange` or `SetFilter` placed before `FindSet` narrows the result set at th
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Move every predicate that can be expressed as an equality or range filter into a `SetRange` or `SetFilter` ahead of the find. Combine with `SetCurrentKey` to choose a key whose first fields match the filter (see `setcurrentkey-aligns-key-with-filters.md`). The loop body should then contain only the work that depends on per-row state.
|
Move every predicate that can be expressed as an equality or range filter into a `SetRange` or `SetFilter` ahead of the find. Make sure a key (index) exists whose leading fields cover the filter so the optimizer can seek; note that `SetCurrentKey` only sets sort order and is not an index hint (see `setcurrentkey-sets-sort-order-not-index-hint.md`). The loop body should then contain only the work that depends on per-row state.
|
||||||
|
|
||||||
See sample: `apply-filters-before-iterating.good.al`.
|
See sample: `apply-filters-before-iterating.good.al`.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,15 +0,0 @@
|
||||||
codeunit 50230 "Perf Sample SetCurrentKey Good"
|
|
||||||
{
|
|
||||||
procedure ProcessLines(var SalesHeader: Record "Sales Header")
|
|
||||||
var
|
|
||||||
SalesLine: Record "Sales Line";
|
|
||||||
begin
|
|
||||||
SalesLine.SetCurrentKey("Document Type", "Document No.", "Line No.");
|
|
||||||
SalesLine.SetRange("Document Type", SalesHeader."Document Type");
|
|
||||||
SalesLine.SetRange("Document No.", SalesHeader."No.");
|
|
||||||
if SalesLine.FindSet() then
|
|
||||||
repeat
|
|
||||||
// ...
|
|
||||||
until SalesLine.Next() = 0;
|
|
||||||
end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,24 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: performance
|
|
||||||
keywords: [setcurrentkey, key, index, filter, sort]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Pick a key whose fields cover the filter and sort with SetCurrentKey
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
The platform chooses a key for each record access. When the filters or required sort do not match the primary key — or any non-explicit choice — the query may run against a key that does not cover the filter columns. Per the upstream guidance, "Use `SetCurrentKey()` to select the most efficient key for your filters" and "match key fields to your filter/sort requirements." Filtering on fields that are not in any key is flagged as bad — there is no index to ride and the access ends up reading more than necessary.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
When the access pattern is anything other than primary-key lookup, look at the filters and the desired sort, then either pick an existing key whose leading fields cover them and call `SetCurrentKey(...)`, or declare a new key on the table for the pattern. Match leading fields first — a key starting with `"Document Type", "Document No.", "Line No."` serves a filter on those three; a key starting with `"Line No."` does not.
|
|
||||||
|
|
||||||
See sample: `setcurrentkey-aligns-key-with-filters.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Applying filters on fields that no key indexes, leaving the platform to read more than it should. The query produces the right answer; the cost surfaces only at production volume. The mirror case is forgetting `SetCurrentKey` when the wanted sort differs from the primary key — the iteration may then be sorted in memory after a wider read than necessary.
|
|
||||||
|
|
@ -0,0 +1,20 @@
|
||||||
|
codeunit 50231 "Perf Sample SetCurrentKey Bad"
|
||||||
|
{
|
||||||
|
// Misconception: SetCurrentKey does NOT tell SQL Server to use this index.
|
||||||
|
// The optimizer picks the index from the filters (WHERE clause) and statistics.
|
||||||
|
// The result order is never used here, so SetCurrentKey only adds an ORDER BY
|
||||||
|
// the query does not need — and can push the plan toward a sort.
|
||||||
|
procedure SumRemainingAmount(CustomerNo: Code[20]) Total: Decimal
|
||||||
|
var
|
||||||
|
CustLedgerEntry: Record "Cust. Ledger Entry";
|
||||||
|
begin
|
||||||
|
CustLedgerEntry.SetCurrentKey("Customer No.", Open, "Posting Date");
|
||||||
|
CustLedgerEntry.SetRange("Customer No.", CustomerNo);
|
||||||
|
CustLedgerEntry.SetRange(Open, true);
|
||||||
|
CustLedgerEntry.SetAutoCalcFields("Remaining Amount");
|
||||||
|
if CustLedgerEntry.FindSet() then
|
||||||
|
repeat
|
||||||
|
Total += CustLedgerEntry."Remaining Amount";
|
||||||
|
until CustLedgerEntry.Next() = 0;
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,18 @@
|
||||||
|
codeunit 50230 "Perf Sample SetCurrentKey Good"
|
||||||
|
{
|
||||||
|
// SetCurrentKey is used because the rows must be processed oldest-first.
|
||||||
|
// The sort is a functional requirement, so the ORDER BY it adds is justified.
|
||||||
|
procedure ApplyOldestEntriesFirst(CustomerNo: Code[20])
|
||||||
|
var
|
||||||
|
CustLedgerEntry: Record "Cust. Ledger Entry";
|
||||||
|
begin
|
||||||
|
CustLedgerEntry.SetRange("Customer No.", CustomerNo);
|
||||||
|
CustLedgerEntry.SetRange(Open, true);
|
||||||
|
CustLedgerEntry.SetCurrentKey("Posting Date");
|
||||||
|
if CustLedgerEntry.FindSet() then
|
||||||
|
repeat
|
||||||
|
// Apply entries in posting-date order ...
|
||||||
|
until CustLedgerEntry.Next() = 0;
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
@ -0,0 +1,35 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: performance
|
||||||
|
keywords: [setcurrentkey, sort, order-by, index, key, query-optimizer, hint]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# SetCurrentKey only sets sort order — it is not an index hint
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
A common misconception is that `SetCurrentKey` tells SQL Server which index to use for a query. It does not. In Business Central, `SetCurrentKey` only changes the `ORDER BY` clause of the generated SQL statement. It does not add an index hint, and the SQL Server query optimizer is free to ignore the named key entirely.
|
||||||
|
|
||||||
|
The optimizer picks the index from the `WHERE` clause (your `SetRange`/`SetFilter`) together with table statistics and estimated cost. In practice it almost never chooses an index just because that key appears in `ORDER BY`. So calling `SetCurrentKey` to "steer" the plan toward an index is a no-op for index selection — and can make things worse: an `ORDER BY` that the query does not otherwise need can push the optimizer toward a less selective index or add a Sort operator to the plan.
|
||||||
|
|
||||||
|
Selectivity comes from having the right index available (a key on the table whose leading fields cover the filter) and from filtering on those fields — not from `SetCurrentKey`.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Decide `SetCurrentKey` on one question only: **do I need the result set in a specific order?**
|
||||||
|
|
||||||
|
- If yes — you iterate rows in a defined sequence, or rely on `FindFirst`/`FindLast`/`Next` returning a particular row — call `SetCurrentKey` for that sort. The order is a functional requirement, and the `ORDER BY` is justified.
|
||||||
|
- If no — omit `SetCurrentKey`. Let the optimizer choose the cheapest plan for your filters; it may pick a better index and skip a sort.
|
||||||
|
|
||||||
|
To make a filtered read fast, ensure a key (index) exists on the table whose leading fields cover the filter, and filter on those fields with `SetRange`/`SetFilter`. That is what lets the optimizer seek. Defining the key creates the index; `SetCurrentKey` is not required to make the optimizer use it.
|
||||||
|
|
||||||
|
See sample: `setcurrentkey-sets-sort-order-not-index-hint.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Adding `SetCurrentKey` to a filtered read purely in the belief that it forces SQL Server to seek a particular index, when the code never uses the resulting order. This does nothing for index selection and only appends an `ORDER BY` the query does not need, risking an unnecessary sort. Remove the `SetCurrentKey`; rely on the filters and an existing covering key instead.
|
||||||
|
|
||||||
|
See sample: `setcurrentkey-sets-sort-order-not-index-hint.bad.al`.
|
||||||
Loading…
Add table
Add a link
Reference in a new issue