Slice 611704: With Autoincrement Tax Transaction Value ID field changed to BigInteger to prevent IDENTITY exhaustion - #10510
Conversation
github-actions
Bot
added
AL: Apps (W1)
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| field(18; "ID"; Integer) | ||
| #pragma warning disable AS0146 | ||
| #pragma warning disable AS0041 | ||
| field(18; "ID"; BigInteger) |
There was a problem hiding this comment.
Table 20261 "Tax Transaction Value" field 18 "ID" changes type from Integer to BigInteger in place (kept AutoIncrement = true), wrapped in #pragma warning disable AS0146 / AS0041 to suppress the analyzer rules that specifically guard against changing a published field's type/AutoIncrement semantics. Suppressing those analyzer warnings rather than following the obsoletion lifecycle (new field + Obsolete on the old one) is a real breaking-change risk for any extension compiled against or persisting this field as Integer.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| field(18; "ID"; Integer) | ||
| #pragma warning disable AS0146 | ||
| #pragma warning disable AS0041 | ||
| field(18; "ID"; BigInteger) |
There was a problem hiding this comment.
Table 20261 field 18 "ID" keeps AutoIncrement = true while the new OnInsert trigger also assigns Rec.ID from SequenceNoMgt.GetNextSeqNoBigInt. Every other table in this codebase whose key/ID field is populated through SequenceNoMgt.GetNextSeqNo* (e.g. "Sent Notification Entry", "Interaction Log Entry") does NOT mark that field AutoIncrement, precisely because the two ID-assignment mechanisms are meant to be mutually exclusive. Leaving AutoIncrement = true alongside the manual sequence assignment deviates from that established convention and, if the impact of the interaction turns out to be functionally significant (e.g. duplicate/gapped IDs across the two counters), this should be escalated to a major/blocker defect and a corresponding knowledge rule authored — remove AutoIncrement from the field definition to match the rest of the codebase.
Agent judgement — not directly backed by a BCQuality knowledge article.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| exit(NewSeqNo); | ||
| ClearLastError(); | ||
| CreateNewTableSequence(PreviewMode, TableNo); | ||
| TryGetNextNoBigInt(PreviewMode, TableNo, NewSeqNo); |
There was a problem hiding this comment.
This standalone call ignores the Boolean result of [TryFunction] TryGetNextNoBigInt, so a failure after CreateNewTableSequence is handled as an ordinary throwing call instead of try-method logic. Consume the return value and handle false explicitly.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
if not TryGetNextNoBigInt(PreviewMode, TableNo, NewSeqNo) then
Error(GetLastErrorText());Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| exit(CurrSeqNo); | ||
| ClearLastError(); | ||
| CreateNewTableSequence(PreviewMode, TableNo); | ||
| TryGetCurrentNoBigInt(PreviewMode, TableNo, CurrSeqNo); |
There was a problem hiding this comment.
This standalone call ignores the Boolean result of [TryFunction] TryGetCurrentNoBigInt, so a failure after CreateNewTableSequence is handled as an ordinary throwing call instead of try-method logic. Consume the return value and handle false explicitly.
Suggested fix (apply manually — could not be anchored as a one-click suggestion):
if not TryGetCurrentNoBigInt(PreviewMode, TableNo, CurrSeqNo) then
Error(GetLastErrorText());Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| } | ||
| } | ||
|
|
||
| trigger OnInsert() |
There was a problem hiding this comment.
The new Tax Transaction Value.OnInsert trigger assigns ID through SequenceNoMgt.GetNextSeqNoBigInt() per insert, and this PR switches the bulk-copy path (TaxDocumentGLPosting.TransferTransactionValue) and TaxRateComputation.InsertTaxTransactionValue to Insert(true) without pre-reserving a sequence buffer, adding one number-sequence lookup per row for high-volume tax posting. Consider SequenceNoMgt.AllocateSeqNoBuffer before the row loop for the bulk-copy path.
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| /// Use for tables whose primary key number field is a BigInteger. If the sequence does not exist, it will be created. | ||
| /// </summary> | ||
| /// <param name="TableNo">The ID of the table being checked</param> | ||
| procedure GetNextSeqNoBigInt(TableNo: Integer): BigInteger |
There was a problem hiding this comment.
The new public BigInteger sequence APIs (GetNextSeqNoBigInt, ValidateSeqNoBigInt, GetCurrentSeqNoBigInt) accept an arbitrary TableNo and route it into the existing GetLastEntryNoFromTable(), which does RecordRef.Open(TableNo) on the caller-controlled table without an allow-list. This mirrors the same shape as the pre-existing GetNextSeqNo/ValidateSeqNo Integer variants in this codeunit, so it is a continuation of an existing pattern rather than a newly introduced flaw, but it is still worth flagging on the new public surface: restrict these procedures' callers or validate TableNo before opening the table.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
| exit(CurrSeqNo); | ||
| end; | ||
|
|
||
| /// <summary> |
There was a problem hiding this comment.
GetNextSeqNoBigInt and GetCurrentSeqNoBigInt add new public return-value procedures to the Sequence No. Mgt. library codeunit, but their XML doc stops at <param> and omits a <returns> tag, unlike similar existing procedures.
Knowledge:
👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4
1 participant
AB#611704
Slice 611704: [Repair Item][IN] Change "Tax Transaction Value".ID to be BigInteger
Issue:
The primary key field ID of table Tax Transaction Value (20261) was Integer with AutoIncrement. On a production environment (IN localization, ~525 GB database) the SQL IDENTITY sequence exceeded the integer limit (2,147,483,647), causing the error: "Arithmetic overflow error converting IDENTITY to data type int." The actual row count (~285M) was well below the limit, but Posting Preview and failed/rolled-back transactions kept consuming AutoIncrement IDs without committing rows, exhausting the sequence far beyond the actual data.
Cause:
SQL Server IDENTITY (AutoIncrement) is non-transactional — every insert attempt permanently increments the counter, even when the transaction is rolled back (Posting Preview, validation errors). Heavy use of Posting Preview consumed the integer range over time. The Integer type has a maximum of only 2,147,483,647.
Solution:
Changed field ID from Integer to BigInteger, extending the range to 9.2 × 10¹⁸. Introduced the platform NumberSequence API for ID generation while retaining AutoIncrement on the field. Centralized ID generation in the existing helper Transaction Value Helper (20236) as GetNextTransactionValueID() — no new object introduced. The sequence is lazily seeded from the current MAX(ID) on first use (collision-free even on an upgraded database), with a TryFunction + ClearLastError guard for concurrent creation. The table OnInsert trigger assigns the ID from the sequence (skipping temporary records and explicitly-set IDs). Live-table insert call sites (TaxRateComputation, TaxDocumentGLPosting) use Insert(true) so the trigger fires. The temp-table NextID variable in TaxPostingBufferMgmt was changed to BigInteger to match the field type. Temporary table inserts manage their own IDs via simple counters and are unaffected by this change.