fix(ntx): split internal nodes correctly in a deep index (off-by-one)

[Admnwk]

Problem

Once an NTX index grows a third level — an internal (branch) node fills up and has to split, which happens around 1500+ keys for a 15-byte key — the build produced a slightly corrupt tree: a full Skip walk returned more rows than exist (e.g. 2003 vs 2000), keys came back out of order, and a few Seeks missed. Unlike the duplicate-key case (#86), this hits plain ascending unique keys, so any large index is affected.

Root cause

In insert(), the internal-node split builds the parent's entry list, inserts the promoted separator at pos, then wired the new right sibling in with:

ents[pos + 1].lchild = prop_right;

For an ascending key stream the descent always lands at the rightmost slot (pos == key_count), so the separator is appended at the end and ents[pos + 1] reads one past the vector. Worse, prop_right was never recorded as the node's rightmost child — the stale original sentinel was reused instead, which for a rightmost leaf split aliases prop_left. So prop_left ended up referenced twice and prop_right was orphaned, injecting one phantom / out-of-order entry per internal split.

Fix

When the separator is appended at the end, prop_right becomes the node's new rightmost child (mirroring the non-splitting "room in parent" path); otherwise the original sentinel is preserved. This also removes the one-past-the-end vector access.

std::uint32_t node_rightmost;
if (pos + 1 < ents.size()) {
    ents[pos + 1].lchild = prop_right;   // separator before the end
    node_rightmost = right_sentinel;
} else {
    node_rightmost = prop_right;          // separator appended at the end
}
...
fill_internal(parent_right, p_right, node_rightmost);

Validation

• New regression test tests/unit/abi_ntx_internal_split_test.cpp: builds a 2000-key unique index (no duplicates, no PACK) and asserts a full Skip walk visits exactly N records in order and every key is reachable by Seek.
  • RED before the fix: walk 2003/2000, out of order, Seek 1943/2000.
  • GREEN after.
• Verified by an independent repro at N up to 10000 (unique and duplicate streams): every record reachable, in order.
• Full unit suite: 786/786, 0 failures. Builds clean under -DOPENADS_WARNINGS_AS_ERRORS=ON (MSVC x64).

Notes

• Stacked on fix(ntx): build a complete multi-page index over runs of duplicate keys #86 (the duplicate-key fix). The first commit here is fix(ntx): build a complete multi-page index over runs of duplicate keys #86; once that merges this collapses to the single internal-split commit. Both touch only src/drivers/ntx/ntx_index.{cpp,h} in different regions and are logically independent.
• Unrelated pre-existing observation: abi_adi_multilevel_build_test is order-dependent — it fails when run in isolation (-tc=...) and passes inside the full suite, on plain v1.4.0 before any of these changes. It's an AdsCreateIndex61 → 5000 on an 8000-row ADI build that depends on warm heap state; not introduced or affected by this change (this fix in fact removes a one-past-the-end write). Flagging it as a separate item worth a look.

[gemini-code-assist]

Code Review

This pull request fixes critical bugs in the NTX B-tree index implementation related to duplicate key insertion and internal node splitting, which previously caused orphaned subtrees and out-of-order/missing keys. It introduces a descend_to_leaf flag to ensure insertions always reach a leaf node, and corrects the handling of the rightmost child sentinel during internal node splits. Two regression tests are added to cover these scenarios. The review feedback highlights potential buffer overflow vulnerabilities in the newly added test files where fixed-size buffers are populated using std::memcpy without bounds checking, and suggests safer alternatives using std::snprintf or assertions.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Using std::memcpy with std::strlen to copy into a fixed-size buffer fld of size 32 can lead to a buffer overflow if field exceeds 31 characters. It is safer to use std::snprintf to ensure null-termination and prevent out-of-bounds writes.

Suggested change

	UNSIGNED8 fld[32];
	std::memcpy(fld, field, std::strlen(field) + 1);
	UNSIGNED8 fld[32];
	std::snprintf(reinterpret_cast<char*>(fld), sizeof(fld), "%s", field);

[gemini-code-assist]

Copying dir.string() into a fixed-size buffer srv of size 256 using std::memcpy can cause a buffer overflow if the temporary directory path is extremely long (which can happen in some deep CI or Windows environments). Adding a REQUIRE check ensures that the path fits within the buffer limits.

Suggested change

	UNSIGNED8 srv[256];
	std::memcpy(srv, dir.string().c_str(), dir.string().size() + 1);
	UNSIGNED8 srv[256];
	REQUIRE(dir.string().size() < sizeof(srv));
	std::memcpy(srv, dir.string().c_str(), dir.string().size() + 1);

[gemini-code-assist]

Using std::memcpy with std::strlen to copy into a fixed-size buffer fld of size 32 can lead to a buffer overflow if field exceeds 31 characters. It is safer to use std::snprintf to ensure null-termination and prevent out-of-bounds writes.

Suggested change

	UNSIGNED8 fld[32];
	std::memcpy(fld, field, std::strlen(field) + 1);
	UNSIGNED8 fld[32];
	std::snprintf(reinterpret_cast<char*>(fld), sizeof(fld), "%s", field);

[gemini-code-assist]

Copying dir.string() into a fixed-size buffer srv of size 256 using std::memcpy can cause a buffer overflow if the temporary directory path is extremely long (which can happen in some deep CI or Windows environments). Adding a REQUIRE check ensures that the path fits within the buffer limits.

Suggested change

	UNSIGNED8 srv[256];
	std::memcpy(srv, dir.string().c_str(), dir.string().size() + 1);
	UNSIGNED8 srv[256];
	REQUIRE(dir.string().size() < sizeof(srv));
	std::memcpy(srv, dir.string().c_str(), dir.string().size() + 1);

[Admnwk]

Merged directly to main as b0ee044 (stacked on #86's c5ce0d5; both cherry-picked onto current main). Full unit suite 800/800, 0 failures, -Werror clean. Note: the pre-existing abi_adi_multilevel_build_test order-dependence flagged above is unaffected (passes in the full suite). Closing as merged — thanks!