Skip to content

Three CDX/AOF correctness fixes from a Harbour/FiveWin ERP deployment #126

Description

@russimicro

Context

We run OpenADS as a drop-in openace64.dll (x64) replacement for Sybase/SAP
ace64.dll under a large Harbour 3.2 + FiveWin ERP, through the stock rddads
RDD with local CDX tables (cTipSer = ADSLOC). While moving real customer
data off SAP ADS we hit three independent correctness bugs. Each has a clear
root cause and a regression test. Sharing them in case you'd like to take them
upstream.

All three are reproduced by unit tests in our fork
(russimicro/OpenADS, branch russoft-erp-compat). Happy to open focused PRs
per fix if you prefer.


Bug 1 — CdxIndex::list_tags() returns tags alphabetically, not in creation order

Symptom. DBSETORDER(n) selects the wrong tag for CDX files created by
SAP ADS (or BCC-built Harbour DBFCDX). In a multi-tag bag, DBSETORDER(1)
picked the alphabetically-first tag instead of the first-created one, so a
TBrowse "sort by column" navigated the wrong key.

Root cause. list_tags() returned tag names in the order of the
structure-tag leaf, which CDX stores sorted alphabetically by tag name.
SAP ADS numbers tags by creation order (the tag-header offset).

Fix. Before returning, stable_sort the struct-leaf entries by the
tag-header offset (creation order). No-op for CDX files created by OpenADS
itself (their struct leaf is already in creation order); only re-orders bags
written by other engines.

Test. tests/unit/abi_cdx_tag_order_test.cpp (fixture CDX built with
Harbour DBFCDX, tags created non-alphabetically; fails before, passes after).


Bug 2 — Expression-index key size truncation + RNBits undersizing (index corrupt after reindex on large tables)

Symptom. After rebuilding a CDX with an expression key over a large
table — e.g. INDEX ON UPPER(cName) TAG TNAME on ~34k rows — navigating that
order returns rows out of key order: the keys are stored sorted but pinned
to the wrong record numbers (~46% mismatched in our data). A plain
field-name key on the same table (INDEX ON cCode TAG TCODE) is fine. Small
tables build the same expression index correctly, so it only shows at scale.

Root cause (two compounding issues).

  1. Key size collapses to the first record's content width.
    AdsCreateIndex61 derives the key length by evaluating the expression on
    the first record and then rtrim-ing the result
    (while (!s.empty() && s.back()==' ') s.pop_back();). Because the field
    read path (make_string) already strips a Character field's trailing
    blanks, the key size becomes the content length of record Steps to create a Mingw ace32.dll & ace64.dll #1 (e.g. 26)
    instead of the field's declared width (e.g. 60). Every longer key is then
    truncated to that width, many become identical, and the bulk-build sort
    tie-breaks equal keys by recno — so keys stay ordered but their recnos are
    scrambled.

  2. RNBits too small for the row count. compute_layout() derives
    req_byte from the key length (b_bits) only, never from the record
    count. For a wide key this yields e.g. rn_bits = 12 (max recno 4095), and
    the packed record number is masked (recno & 4095) on any table larger
    than that.

Fix.

  • Evaluate index-key fields at their declared fixed width (re-pad after
    make_string trims), so UPPER(field) / field1+field2 keys are
    constant-length regardless of the row.
  • Derive the index key size from the expression's natural fixed-width
    length
    (no rtrim).
  • Size req_byte so RNBits covers the max recno, mirroring Harbour
    hb_cdxPageLeafInitSpace: ReqByte = ceil((RNBits + 2*bBits)/8).

FOR-clause / scope predicates are unaffected (they use a separate evaluator).

Tests. tests/unit/abi_cdx_expr_index_scale_test.cpp (30k rows,
UPPER() key with long shared prefixes; fails with thousands of rows out of
order before the fix, passes after) and tests/unit/abi_multitag_order_nav_test.cpp.


Bug 3 — AdsSetAOF returning success for a non-optimisable filter silently disables SET FILTER

Symptom. A Clipper SET FILTER TO field >= m_lo .AND. field <= m_hi
(host variables, not literals) is a no-op — the whole table is walked.

Root cause. rddads translates SET FILTER into
AdsSetAOF(filterText, ADS_RESOLVE_DYNAMIC), passing the filter text with the
unresolved memory variables (field >= m_lo ...). OpenADS' AOF parser
correctly declines (not optimisable), but AdsSetAOF then returned
AE_SUCCESS (with OPTIMIZED_NONE). The catch: stock rddads decides whether
to run its own client-side row filter purely from AdsSetAOF's return
value — it does not call AdsGetAOFOptLevel. So on AE_SUCCESS it assumes
the server optimised the filter and skips the client-side pass. Net effect:
no filtering at all, server-side or client-side.

Fix. AdsSetAOF returns a non-success code (we used
AE_INVALID_EXPRESSION) when the expression cannot be turned into a
server-side AOF. rddads then falls back to its client-side filter, which
works because AdsGetField already returns Character fields at their
fixed width (so the Harbour filter codeblock compares correctly). This matches
real ADS, which errors when an AOF cannot be built. (Returning
"success + OPTIMIZED_NONE" reads correct in isolation, but with stock rddads it
silently turns SET FILTER off.)

Test. tests/unit/abi_aof_test.cpp (non-optimisable AOF now returns a
non-success code; the RDD is expected to filter client-side).


Status / offer

  • Full unit suite green (726 cases) with the three fixes.
  • These are engine/ABI correctness fixes, independent of our ERP-specific
    dialect work.
  • If useful, we can open a focused PR per fix rebased onto main. Note our
    build branch has drifted from main, so the diffs above are the cleanest way
    to review the intent; the tests pin each behaviour.

Branch with all three (plus tests): russimicro/OpenADS russoft-erp-compat.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions