Skip to content

Bug 611704: Align Tax Transaction Value ID assignment with G/L Entry pattern - #10660

Open
v-maheshsharma wants to merge 1 commit into
mainfrom
bugs/Bug-611704-GLEntryStyle-TaxTransactionValue-ID-Field-with-AutoIncrement
Open

v-maheshsharma wants to merge 1 commit into
mainfrom
bugs/Bug-611704-GLEntryStyle-TaxTransactionValue-ID-Field-with-AutoIncrement

Conversation

@v-maheshsharma

@v-maheshsharma v-maheshsharma commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

AB#611704
Slice 611704: [Repair Item][IN] Change "Tax Transaction Value".ID to be BigInteger

Issue

The ID field on the Tax Transaction Value table (20261) is assigned solely via SQL IDENTITY (AutoIncrement). Due to high churn (delete + re-insert on every tax recompute) and non-transactional IDENTITY consumption (numbers burned on Posting Preview and rollbacks), the ID column overflowed, producing "Arithmetic overflow error converting IDENTITY to data type int" (AB#611704).

Cause

ID generation depended only on the database IDENTITY column — there was no application-managed sequence and no alignment with the platform's standard ledger-entry numbering pattern used by G/L Entry.

Solution

Introduced G/L Entry–style ID assignment:

  • TaxTransactionValue.Table.al — added a new GetNextID() procedure that returns the next number from Sequence No. Mgt., mirroring G/L Entry's GetNextEntryNo(). The field is left unchanged (Integer, AutoIncrement = true) — no schema/breaking change.
  • TaxRateComputation & TaxDocumentGLPosting — assign ID := GetNextID() before Insert().

Tested: Purchase Invoice posting completes successfully with no errors.

@v-maheshsharma
v-maheshsharma requested a review from a team August 26, 2026 12:43
@v-maheshsharma
v-maheshsharma requested a review from a team as a code owner August 26, 2026 12:43
@github-actions github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label Aug 26, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Aug 26, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 1

Recommendation: Request Changes

What this PR does

This change moves Tax Transaction Value ID assignment away from SQL identity values by assigning Sequence No. Mgt. numbers before persistent inserts. That targets the reported overflow caused by repeated tax value delete/reinsert cycles and rollback or preview flows. The insert paths are covered, but the new sequence helper still uses the normal table sequence during posting preview because table 20261 is not marked as previewable, so the main source of burned numbers is still present.

Problem-solution fit

Fit: Partial

The change matches the reported table and the high-churn insert paths, but it does not fully match the rollback and posting preview part of the problem. Preview can still consume the persistent sequence, so the fix reduces reliance on SQL identity but does not fully remove the overflow path.

Suggestions

S1 (🔴 High): Use a preview sequence for tax values
The new GetNextID() calls Sequence No. Mgt. for table 20261, but no OnPreviewableLedgerEntry subscriber marks this table as previewable. Posting preview can still burn the normal sequence and drive the integer ID toward overflow. Add the previewable registration and cover the preview path so preview uses the separate negative sequence.

Risk assessment and necessity

Risk: The regression surface is India tax transaction storage during tax calculation, document posting, and transfer of tax values. ID allocation is data-sensitive because a duplicate, exhausted, or skipped persistent sequence can stop posting or tax recomputation; the change does not alter tax amounts directly, but it sits on a posting-adjacent path.

Necessity: The change is necessary because the table is churned heavily and identity values can be consumed even when data is rolled back. The scope is close to the right G/L Entry pattern, but it must include the preview-sequence part of that pattern to solve the reported failure completely.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=10660 round=1 by=alexei-dobriansky at=2026-08-27T13:16:00Z lastSha=440ebe9e13f57dd666e14677ba299d0a7e98455d reviewKey=ac15e863909d5c2ff7c449eb081edc10c15bfec74b466b91d330ab58db09344a suggestions=S1@503d3bcc

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.


🔄 How to trigger a new status check:

  1. 📤 Push a new commit to the PR branch, or
  2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 Team: Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants