Skip to content

fix(engine): seek_key must honor SET DELETED ON #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Admnwk wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(engine): seek_key must honor SET DELETED ON #4

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b31d421
fix(engine): seek_key must honor SET DELETED ON
nllf Jun 18, 2026
670adc7
fix(engine): seek_key walk_to_last honors SET DELETED ON
Jun 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 53 additions & 8 deletions src/engine/table.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,33 @@ Table::seek_key(const std::string& key, bool soft, bool last) {
return false;
}
bool exact = r.value().hit == drivers::SeekHit::Exact;
// SET DELETED ON: a seek must not report a deleted row as found.
// goto_top/goto_bottom/skip already honor show_deleted() but seek_key
// did not (the B+tree match landed on a deleted row and was returned
// as found). Skip forward over deleted records at the landing, then
// re-derive `exact` from the row we actually land on (Clipper/DBFCDX:
// if the only matching rows are deleted, Found() is .F. and the cursor
// sits on the next live record or Eof).
if (!openads::abi::show_deleted()) {
std::string del_key = key;
del_key.resize(idx->key_length(), ' ');
while (r.value().positioned) {
if (auto ld = load_record_(r.value().recno); !ld) return ld.error();
if (!is_deleted()) break;
r = idx->next();
if (!r) return r.error();
}
if (!r.value().positioned) {
state_ = (driver_->record_count() == 0) ? State::Limbo
: State::Eof;
recno_ = 0;
last_seek_found_ = false;
return false;
}
std::string ck = idx->current_key();
ck.resize(del_key.size(), ' ');
exact = (ck == del_key);
}

@gemini-code-assist gemini-code-assist Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

When SET DELETED ON is active, if the seek lands on an exact match and walk_to_last is true (which is always the case for descending indexes or when last is true), the code walks forward to the last matching key without checking if those records are deleted. If the last matching record is deleted, the seek will land on it and report it as found, violating SET DELETED ON.

To fix this, after the walk_to_last loop finishes and loads the record, if SET DELETED ON is active and the loaded record is deleted, we should walk backward using idx->prev() until we find a non-deleted record. Since we started at a known active matching record, we are guaranteed to find one.

Here is how the walk_to_last block (lines 1083-1109) should be updated:

    if (walk_to_last && exact) {
        std::string padded_key = key;
        padded_key.resize(idx->key_length(), ' ');
        std::uint32_t last_recno = r.value().recno;
        while (true) {
            auto step = idx->next();
            if (!step || !step.value().positioned) break;
            std::string ck = idx->current_key();
            ck.resize(padded_key.size(), ' ');
            if (ck != padded_key) {
                (void)idx->prev();
                break;
            }
            last_recno = step.value().recno;
        }
        auto load = load_record_(last_recno);
        if (!load) return load.error();
        if (!openads::abi::show_deleted()) {
            while (is_deleted()) {
                auto step = idx->prev();
                if (!step || !step.value().positioned) break;
                if (auto ld = load_record_(step.value().recno); !ld) return ld.error();
            }
        }
        last_seek_found_ = true;
        return true;
    }

Comment on lines +1049 to +1068

@gemini-code-assist gemini-code-assist Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The manual padding and resizing of del_key and ck can be simplified using std::string::resize(size, fill_char). This reduces boilerplate and makes the code much more readable. Additionally, we can use operator== for string comparison instead of std::memcmp.

    if (!openads::abi::show_deleted()) {
        std::string del_key = key;
        del_key.resize(idx->key_length(), ' ');
        while (r.value().positioned) {
            if (auto ld = load_record_(r.value().recno); !ld) return ld.error();
            if (!is_deleted()) break;
            r = idx->next();
            if (!r) return r.error();
        }
        if (!r.value().positioned) {
            state_ = (driver_->record_count() == 0) ? State::Limbo
                                                    : State::Eof;
            recno_ = 0;
            last_seek_found_ = false;
            return false;
        }
        std::string ck = idx->current_key();
        ck.resize(del_key.size(), ' ');
        exact = (ck == del_key);
    }

// DESCEND order treats the FIRST match in walk direction as
// the LAST entry in the equal-key group when sorted ASC. Walk
// duplicates regardless of `last` flag.
Expand All @@ -1050,19 +1077,14 @@ Table::seek_key(const std::string& key, bool soft, bool last) {
// the last matching entry; load_record_ syncs the table buffer.
if (walk_to_last && exact) {
std::string padded_key = key;
if (padded_key.size() < idx->key_length())
padded_key.append(idx->key_length() - padded_key.size(), ' ');
if (padded_key.size() > idx->key_length())
padded_key.resize(idx->key_length());
padded_key.resize(idx->key_length(), ' ');
std::uint32_t last_recno = r.value().recno;
while (true) {
auto step = idx->next();
if (!step || !step.value().positioned) break;
std::string ck = idx->current_key();
if (ck.size() < padded_key.size())
ck.append(padded_key.size() - ck.size(), ' ');
if (std::memcmp(ck.data(), padded_key.data(),
padded_key.size()) != 0) {
ck.resize(padded_key.size(), ' ');
if (ck != padded_key) {
// Past the equal-key run — step back one to leave
// cursor on the last matching entry.
(void)idx->prev();
Expand All @@ -1072,6 +1094,29 @@ Table::seek_key(const std::string& key, bool soft, bool last) {
}
auto load = load_record_(last_recno);
if (!load) return load.error();
if (!openads::abi::show_deleted()) {
while (is_deleted()) {
auto step = idx->prev();
if (!step || !step.value().positioned) {
state_ = (driver_->record_count() == 0) ? State::Limbo
: State::Eof;
recno_ = 0;
last_seek_found_ = false;
return false;
}
std::string ck = idx->current_key();
ck.resize(padded_key.size(), ' ');
if (ck != padded_key) {
state_ = (driver_->record_count() == 0) ? State::Limbo
: State::Eof;
recno_ = 0;
last_seek_found_ = false;
return false;
}
if (auto ld = load_record_(step.value().recno); !ld)
return ld.error();
}
}
last_seek_found_ = true;
return true;
}
Expand Down