diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index da172ff..6c3c484 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -85,7 +85,8 @@ "use-setloadfields-for-partial-records", "prefer-modifyall-over-per-row-modify", "al-methods-limited-during-write-transactions", - "avoid-user-prompts-inside-transactions" + "avoid-user-prompts-inside-transactions", + "use-grouped-query-for-distinct-values-and-duplicates" ] }, "privacy": { diff --git a/microsoft/knowledge/performance/use-grouped-query-for-distinct-values-and-duplicates.bad.al b/microsoft/knowledge/performance/use-grouped-query-for-distinct-values-and-duplicates.bad.al new file mode 100644 index 0000000..dbfad2c --- /dev/null +++ b/microsoft/knowledge/performance/use-grouped-query-for-distinct-values-and-duplicates.bad.al @@ -0,0 +1,21 @@ +codeunit 50572 "Customer Name Audit" +{ + // The outer loop is unfiltered: one extra filtered Count() per customer. + // The question is about the whole table, not about any single row. + procedure HasDuplicateCustomerNames(): Boolean + var + Customer: Record Customer; + OtherCustomer: Record Customer; + begin + Customer.SetLoadFields(Name); + if Customer.FindSet() then + repeat + if Customer.Name <> '' then begin + OtherCustomer.SetRange(Name, Customer.Name); + if OtherCustomer.Count() > 1 then + exit(true); + end; + until Customer.Next() = 0; + exit(false); + end; +} diff --git a/microsoft/knowledge/performance/use-grouped-query-for-distinct-values-and-duplicates.good.al b/microsoft/knowledge/performance/use-grouped-query-for-distinct-values-and-duplicates.good.al new file mode 100644 index 0000000..813fe4a --- /dev/null +++ b/microsoft/knowledge/performance/use-grouped-query-for-distinct-values-and-duplicates.good.al @@ -0,0 +1,47 @@ +// One row per customer name that occurs more than once. +// Name is a plain column, so it is the grouping key; the ColumnFilter on the +// Count column is applied after grouping (HAVING COUNT(*) > 1). +query 50570 "Duplicate Customer Names" +{ + QueryType = Normal; + + elements + { + dataitem(Customer; Customer) + { + column(Name; Name) + { + ColumnFilter = Name = filter(<> ''); + } + column(NameCount) + { + Method = Count; + ColumnFilter = NameCount = filter(> 1); + } + } + } +} + +codeunit 50571 "Customer Name Review" +{ + procedure HasDuplicateCustomerNames(): Boolean + var + DuplicateCustomerNames: Query "Duplicate Customer Names"; + Found: Boolean; + begin + DuplicateCustomerNames.Open(); + Found := DuplicateCustomerNames.Read(); + DuplicateCustomerNames.Close(); + exit(Found); + end; + + procedure GetDuplicateCustomerNames(var DuplicateNames: List of [Text]) + var + DuplicateCustomerNames: Query "Duplicate Customer Names"; + begin + DuplicateCustomerNames.Open(); + while DuplicateCustomerNames.Read() do + DuplicateNames.Add(DuplicateCustomerNames.Name); + DuplicateCustomerNames.Close(); + end; +} diff --git a/microsoft/knowledge/performance/use-grouped-query-for-distinct-values-and-duplicates.md b/microsoft/knowledge/performance/use-grouped-query-for-distinct-values-and-duplicates.md new file mode 100644 index 0000000..6e206ea --- /dev/null +++ b/microsoft/knowledge/performance/use-grouped-query-for-distinct-values-and-duplicates.md @@ -0,0 +1,39 @@ +--- +bc-version: [all] +domain: performance +keywords: [duplicate, distinct, select-distinct, count, having, group-by, columnfilter, method-count, query] +technologies: [al] +countries: [w1] +application-area: [all] +--- + +# Use a grouped query for distinct values and duplicate detection + +## Description + +The AL `Record` type has no `SELECT DISTINCT` or `GROUP BY ... HAVING`. Code that must find which values of a field occur more than once in a table is therefore often written as a loop over the table that, for every row, filters a second record variable on that row's value and calls `Count()`. That sends one extra SQL statement per looped row, so an unfiltered loop costs as many statements as the table has rows, to answer a question about the whole table. A query object answers it in one statement: when any column has an aggregate `Method`, every other `column` becomes an implicit grouping key, so the dataset has one row per distinct combination. A `ColumnFilter` on a non-aggregated column is applied like a `WHERE` clause; a `ColumnFilter` on an aggregated column is applied like a `HAVING` clause, after grouping. A `Method = Count` column with `ColumnFilter = = filter(> 1)` therefore returns only the duplicate groups. This complements [aggregate-before-persisting-intermediate-results](aggregate-before-persisting-intermediate-results.md), which covers grouped totals. + +## Best Practice + +Declare one plain `column` per field of the combination to check, plus a `column` with `Method = Count` and no source field. For a distinct list, read the rows and ignore the count. For duplicates, set `ColumnFilter` on the count column to `filter(> 1)`; one successful `Read()` proves a duplicate exists. Restrict rows with `SetRange`/`SetFilter` on plain columns, or with a `filter` element, which restricts rows but is not included in the dataset. Every extra `column` changes the grouping grain. A runtime `SetFilter` or `SetRange` on the count column replaces its `ColumnFilter` ([setfilter-overwrites-query-columnfilter](../query/setfilter-overwrites-query-columnfilter.md)). Base Application uses this shape to reject duplicate descriptions, for example query 762 "Acc. Sched. Line Desc. Count". + +See sample: [`use-grouped-query-for-distinct-values-and-duplicates.good.al`](use-grouped-query-for-distinct-values-and-duplicates.good.al). + +## Anti Pattern + +A loop over a table (`FindSet` ... `Next`) that, for each row, sets a filter on a second record variable of the same table, over the same rows, to the current row's value and calls `Count()`, `IsEmpty()`, or `FindFirst()` only to learn whether the value occurs more than once. Do not flag: + +- a single uniqueness check for one record, for example in `OnValidate` or before `Insert`; +- an outer loop bounded to one parent, such as the lines of one document, where the statement count does not grow with the table; +- a lookup against a different subset than the one being looped, for example looping quote lines and looking up contract lines; +- a loop that acts on each row it finds, such as marking or updating it, rather than only answering a table-level question; +- a loop where the record being filtered and counted is temporary, which makes no database calls. + +See sample: [`use-grouped-query-for-distinct-values-and-duplicates.bad.al`](use-grouped-query-for-distinct-values-and-duplicates.bad.al). + +## References + +- [Aggregating data in query objects](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/devenv-query-totals-grouping): an aggregate `Method` groups the dataset by the other columns; a `Count` column takes only a name; "Using a query to get distinct values". +- [Filtering in query objects](https://learn.microsoft.com/dynamics365/business-central/dev-itpro/developer/devenv-query-filters): `ColumnFilter` can be applied to aggregated columns; filters on columns with a totals method correspond to a `HAVING` clause, others to `WHERE`; a filter row is not included in the dataset. +- Base Application: [AccSchedLineDescCount.Query.al](https://github.com/microsoft/BCApps/blob/837ef802485ee457e52310d2ecaa08b93d0122fd/src/Layers/W1/BaseApp/Finance/FinancialReports/AccSchedLineDescCount.Query.al#L15-L37) (query 762), used by `CheckDuplicateAccScheduleLineDescription` in [AccSchedChartManagement.Codeunit.al](https://github.com/microsoft/BCApps/blob/837ef802485ee457e52310d2ecaa08b93d0122fd/src/Layers/W1/BaseApp/Finance/FinancialReports/AccSchedChartManagement.Codeunit.al#L390-L398). The same shape appears in [ColmLaytColmHeaderCount.Query.al](https://github.com/microsoft/BCApps/blob/837ef802485ee457e52310d2ecaa08b93d0122fd/src/Layers/W1/BaseApp/Finance/FinancialReports/ColmLaytColmHeaderCount.Query.al) and [Inventory/Analysis/AnalysisLineDescCount.Query.al](https://github.com/microsoft/BCApps/blob/837ef802485ee457e52310d2ecaa08b93d0122fd/src/Layers/W1/BaseApp/Inventory/Analysis/AnalysisLineDescCount.Query.al). +- A legitimate per-row lookup that this rule must not flag: [ServiceContractHeader.Table.al](https://github.com/microsoft/BCApps/blob/837ef802485ee457e52310d2ecaa08b93d0122fd/src/Layers/W1/BaseApp/Service/Contract/ServiceContractHeader.Table.al#L2692-L2705) loops one quote's lines, looks up contract lines for each service item, and marks each hit. diff --git a/microsoft/skills/review/al-performance-review.md b/microsoft/skills/review/al-performance-review.md index 6eae659..1fc1139 100644 --- a/microsoft/skills/review/al-performance-review.md +++ b/microsoft/skills/review/al-performance-review.md @@ -47,6 +47,7 @@ Apply these targeted cues even when simple token overlap would rank the article - Worklist `design-covering-keys-from-read-pattern.md` for a changed secondary key alongside a filtered reader of its table, and `review-overlapping-keys-before-adding-an-index.md` when added or changed keys have overlapping leading fields. A changed key alone is not a finding; read and write workloads determine whether either rule applies. - Worklist `preserve-buffered-inserts-by-separating-target-reads.md` when a loop calls `Insert` and interleaves operations on the insert target or `Commit`. Worklist `aggregate-before-persisting-intermediate-results.md` when repeated grouping calculations and persistent intermediate summaries appear in the same processing path. Do not infer either pattern from `Insert` or `CalcSums` alone. +- Worklist `use-grouped-query-for-distinct-values-and-duplicates.md` when a record loop sets a filter on a second record variable of the same table to the current row's value and calls `Count`, `IsEmpty`, or `FindFirst` only to decide whether that value is duplicated. Do not worklist it for a single uniqueness check on one record (for example in `OnValidate` or before `Insert`), an outer loop bounded to one parent document, a lookup against a different subset than the looped rows, a loop that acts on each row it finds, or a filtered/counted record that is temporary. - Worklist `cache-repeated-filtered-results-with-explicit-scope.md` only when the code or workload establishes repeated **complete** lookup keys (for example, querying the same filtered set in multiple passes, or measured key reuse), or shows a cache that omits result-affecting inputs. A single loop over possibly distinct keys does not establish reuse or justify a cache finding. Worklist `avoid-repeating-unchanged-validation.md` for repeated `Validate` of the same field in one path; do not worklist it from a single validation call. - Worklist `use-setautocalcfields-for-per-row-flowfields.md` when a record loop calls `CalcFields`, or when every row reads the same FlowField for a comparison, branch, or per-record action. Also worklist `calcsums-instead-of-calcfields-in-loop.md` when the loop accumulates one set total: use `CalcSums` directly only for stored source fields, never directly on FlowFields; a FlowField total requires deriving equivalent source filters from its `CalcFormula`. - Worklist `hidden-flowfields-still-calculate-before-bc26-opt-in.md` when a page control directly sources a FlowField and sets `Visible = false` or a visibility expression. Suppress it when the target is known to have BC26's **Calculate only visible FlowFields** feature enabled, or when the FlowField is cheap and intentionally preloaded.