You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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).
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.
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/OpenADSrussoft-erp-compat.
Context
We run OpenADS as a drop-in
openace64.dll(x64) replacement for Sybase/SAPace64.dllunder a large Harbour 3.2 + FiveWin ERP, through the stockrddadsRDD with local CDX tables (
cTipSer = ADSLOC). While moving real customerdata 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, branchrussoft-erp-compat). Happy to open focused PRsper fix if you prefer.
Bug 1 —
CdxIndex::list_tags()returns tags alphabetically, not in creation orderSymptom.
DBSETORDER(n)selects the wrong tag for CDX files created bySAP 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 thestructure-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_sortthe struct-leaf entries by thetag-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 withHarbour 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 TNAMEon ~34k rows — navigating thatorder 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. Smalltables build the same expression index correctly, so it only shows at scale.
Root cause (two compounding issues).
Key size collapses to the first record's content width.
AdsCreateIndex61derives the key length by evaluating the expression onthe first record and then rtrim-ing the result
(
while (!s.empty() && s.back()==' ') s.pop_back();). Because the fieldread path (
make_string) already strips a Character field's trailingblanks, 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.
RNBits too small for the row count.
compute_layout()derivesreq_bytefrom the key length (b_bits) only, never from the recordcount. For a wide key this yields e.g.
rn_bits = 12(max recno 4095), andthe packed record number is masked (
recno & 4095) on any table largerthan that.
Fix.
make_stringtrims), soUPPER(field)/field1+field2keys areconstant-length regardless of the row.
length (no rtrim).
req_bytesoRNBitscovers the max recno, mirroring Harbourhb_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 oforder before the fix, passes after) and
tests/unit/abi_multitag_order_nav_test.cpp.Bug 3 —
AdsSetAOFreturning success for a non-optimisable filter silently disablesSET FILTERSymptom. 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.
rddadstranslatesSET FILTERintoAdsSetAOF(filterText, ADS_RESOLVE_DYNAMIC), passing the filter text with theunresolved memory variables (
field >= m_lo ...). OpenADS' AOF parsercorrectly declines (not optimisable), but
AdsSetAOFthen returnedAE_SUCCESS(withOPTIMIZED_NONE). The catch: stockrddadsdecides whetherto run its own client-side row filter purely from
AdsSetAOF's returnvalue — it does not call
AdsGetAOFOptLevel. So onAE_SUCCESSit assumesthe server optimised the filter and skips the client-side pass. Net effect:
no filtering at all, server-side or client-side.
Fix.
AdsSetAOFreturns a non-success code (we usedAE_INVALID_EXPRESSION) when the expression cannot be turned into aserver-side AOF.
rddadsthen falls back to its client-side filter, whichworks because
AdsGetFieldalready returns Character fields at theirfixed 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 FILTERoff.)Test.
tests/unit/abi_aof_test.cpp(non-optimisable AOF now returns anon-success code; the RDD is expected to filter client-side).
Status / offer
dialect work.
main. Note ourbuild branch has drifted from
main, so the diffs above are the cleanest wayto review the intent; the tests pin each behaviour.
Branch with all three (plus tests):
russimicro/OpenADSrussoft-erp-compat.