Skip to content

fix(adi): tag ordinals follow creation order, as CDX does - #148

Closed
russimicro wants to merge 1 commit into
FiveTechSoft:mainfrom
russimicro:fix/adi-tagdir-append-order
Closed

fix(adi): tag ordinals follow creation order, as CDX does#148
russimicro wants to merge 1 commit into
FiveTechSoft:mainfrom
russimicro:fix/adi-tagdir-append-order

Conversation

@russimicro

Copy link
Copy Markdown
Collaborator

Reported from a Harbour/FiveWin ERP migrating a company from DBFCDX to ADT.

Problem

AdiIndex::add_tag prepends its tag-directory entry, so tag ordinals come
out reversed. Creating TCODIGO, TNOMBRE, TGRUPO in that order leaves TGRUPO as
ordinal 1.

An xBase application navigates orders by number as much as by name —
OrdSetFocus(n), OrdName(n), DBSETORDER(n) — and a browse that sorts on a
column click resolves the column to a tag number. With reversed ordinals it
activates the wrong order, and it cannot compensate: the mapping depends on how
many tags the bag ends up holding, which the caller does not know while it is
still creating them. Over a .CDX bag the same code gets creation order.

Change

Append the entry instead of prepending it. One block in add_tag.

Reading is unaffected — scan_tagdir walks the directory in file order either
way — so a bag written before this change keeps its current ordinals until it is
rebuilt.

Note on the existing test

abi_adi_tagdir_wide_page_test.cpp deliberately avoids asserting ordinals
("tag ordinals depend on whether the bag prepends or appends its directory
entries") and activates tags by handle. It keeps passing. The new test pins the
ordering that this PR establishes.

Verification

tests/unit/abi_adi_tagdir_order_test.cpp — creates three tags in a known
order, closes, reopens, and checks list_tags reports them in creation order.
Fails before the change (reads {CGRUPO, CNOMBRE, CCODIGO}), passes after.

Full suite on this branch (MSVC x64 Release): 1228/1235. The 7 failures are
pre-existing on main and unrelated — 6 × abi_pritpal_lock_test (remote
connect, the residual noted in the v1.8.37 release) and one
abi_oads_file_funcs_test teardown that threw on remove_all because the
temp directory was held by another process on this machine.

add_tag prepended its tag-directory entry, so ordinals came out reversed:
after creating TCODIGO, TNOMBRE, TGRUPO the bag reported TGRUPO as ordinal
1. An application that navigates orders by NUMBER -- OrdSetFocus(n) /
OrdName(n), which is what a browse doing click-to-sort on a column does --
then activates the wrong order, and it cannot compensate without knowing
in advance how many tags the bag will end up holding. The same application
over a .CDX bag gets creation order.

Append instead. Reading is unaffected (the directory is scanned in file
order either way), so a bag written before this change keeps its old
ordinals until it is rebuilt.

The existing wide-page test deliberately avoids asserting ordinals; the
new test pins them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

FiveTechSoft added a commit that referenced this pull request Jul 30, 2026
Cherry-picked and conflict-resolved against main (post CI fix 54e977c):

- #150 fix(sql): ORDER BY cursor keeps source column types
- #148 fix(adi): tag ordinals follow creation order
- #152 fix(adt): non-.adt extension (.DAT) works end to end
- #151 fix(session): create honours absolute path with existing parent
  (+ #include <vector> so the new unit test builds under clang)
- #154 fix(engine): partial SEEK keeps Found() with SET DELETED ON
- #155 fix(engine): ordScope bound shorter than key is a prefix
- #153 perf(engine): live key count without per-row goto + recno order

Also includes the CMakeLists resolution that keeps every new regression
case in the unit-test target.
@FiveTechSoft

Copy link
Copy Markdown
Owner

Integrated into main as part of the PR stack merge (9e7ac41). Cherry-picked onto post-CI-fix main, conflict-resolved (CHANGELOG / CMakeLists), and verified locally. Thank you @russimicro.

FiveTechSoft pushed a commit that referenced this pull request Jul 30, 2026
RusSoft ERP PR batch (#148#155) + CI unit-test unblock. Release notes in
release-notes-1.8.40.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants