Просмотр исходного кода

perf(scan): evaluate filters on the yyjson tree, materialise only the page

A query with one filter cost 2463ms where the same query without one cost
8ms, because the general scan path built a full Document for every row in
the collection before testing the predicate. Measured over 193 MB of real
rows, materialisation is 88% of that: yyjson parse alone 369ms, full
decode_document 3168ms.

scan() now runs two passes. Pass 1 parses each value with yyjson only and
evaluates the predicate through a field resolver, collecting {id, sort key}
per match; pass 2 decodes only the rows inside the requested page. On
executions (9800 docs / 505 MB) the filtered query drops 2463ms -> 402ms,
against a 369ms parse floor.

The operator semantics stay single-sourced: filter_eval.hpp gains
matchesFiltersResolved(resolve, filters) and matchesFilters(doc, filters)
becomes a wrapper over it, since two copies would drift into silently wrong
results rather than errors. SEARCH needs the whole document, so
needsWholeDocument() routes those queries to the old path and the resolver
path fails closed for them.

Also: encryption_manager logged at INFO on every field encrypt, including
the first and last plaintext byte as hex, leaking characteristics of
sensitiveFields values into the service log at default verbosity. Now DEBUG
without the bytes.

test_filtered_scan_operator_matrix covers all eleven operators, dotted
paths, the four metadata fields, SEARCH and filtered sort/pagination.
ctest 19/19.
fszontagh 1 месяц назад
Родитель
Сommit
82381ff7ec

Разница между файлами не показана из-за своего большого размера
+ 0 - 0
CLAUDE.md


+ 14 - 5
service/src/encryption/encryption_manager.cpp

@@ -280,13 +280,22 @@ std::string EncryptionManager::encryptValue(const std::string& plaintext) {
         return plaintext;
     }
 
-    SPDLOG_INFO("Encrypting value of length {} bytes, first byte={:02x}, last byte={:02x}",
-                plaintext.length(),
-                plaintext.empty() ? 0 : static_cast<uint8_t>(plaintext[0]),
-                plaintext.empty() ? 0 : static_cast<uint8_t>(plaintext[plaintext.length()-1]));
+    // v2.8.0 — DEBUG, and no plaintext.
+    //
+    // This logged twice per encrypted value at INFO, including the first and
+    // last byte of the PLAINTEXT in hex. These are the fields an operator marked
+    // `sensitiveFields` precisely so they would not be readable at rest, and the
+    // log then published characteristics of them - on a low-cardinality field
+    // (a status, a country code, a boolean-ish string) first+last byte plus
+    // length is often enough to identify the value outright.
+    //
+    // Length alone is kept because it is useful for diagnosing size limits and
+    // is already inferable from the ciphertext size.
+    SPDLOG_DEBUG("Encrypting value of length {} bytes", plaintext.length());
     std::vector<uint8_t> plaintextBytes(plaintext.begin(), plaintext.end());
     auto encrypted = encryptData(plaintextBytes);
-    SPDLOG_INFO("Encrypted data size: {} bytes (IV=12 + CT={} + Tag=16)", encrypted.size(), encrypted.size() - 28);
+    SPDLOG_DEBUG("Encrypted data size: {} bytes (IV=12 + CT={} + Tag=16)",
+                 encrypted.size(), encrypted.size() - 28);
 
     return std::string(ENCRYPTED_PREFIX) + base64Encode(encrypted);
 }

+ 182 - 0
service/src/storage/document_store_lmdb.cpp

@@ -23,6 +23,7 @@
 #include "storage/document_store_lmdb.hpp"
 
 #include <lmdb.h>
+#include <yyjson.h>
 
 #include <algorithm>
 #include <cctype>
@@ -104,6 +105,85 @@ smartbotic::database::Document decode_document(std::string_view bytes) {
 // shared with MemoryStore so a shadow-read divergence can never be the
 // result of two private copies drifting apart.
 
+
+// -------------------------------------------------------------------------
+// v2.8.0 — resolve one filter field straight out of a yyjson tree.
+//
+// A filtered scan used to build a full Document per row just to ask whether the
+// row matched. Over 10,124 real rows (193 MB) that was 3168ms against 369ms for
+// the yyjson parse alone - materialising was 88% of the work, and a filtered
+// query on that collection took ~2.5s to return ten documents. Views always add
+// their baked-in filters, so every view query paid it.
+//
+// This converts only the ONE value a filter asks about. The comparison logic
+// still lives in filter_eval, shared with MemoryStore's path.
+//
+// Field names map onto the stored JSON shape written by Document::toJson():
+// metadata at the top level, user fields under "data".
+// -------------------------------------------------------------------------
+std::optional<nlohmann::json> resolve_from_yyjson(yyjson_val* root,
+                                                  const std::string& field) {
+    if (!root || !yyjson_is_obj(root)) return std::nullopt;
+
+    auto to_json = [](yyjson_val* v) -> std::optional<nlohmann::json> {
+        if (!v) return std::nullopt;
+        switch (yyjson_get_type(v)) {
+            case YYJSON_TYPE_NULL: return nlohmann::json(nullptr);
+            case YYJSON_TYPE_BOOL: return nlohmann::json(yyjson_get_bool(v));
+            case YYJSON_TYPE_NUM:
+                if (yyjson_is_real(v)) return nlohmann::json(yyjson_get_real(v));
+                if (yyjson_is_sint(v)) return nlohmann::json(yyjson_get_sint(v));
+                return nlohmann::json(yyjson_get_uint(v));
+            case YYJSON_TYPE_STR:
+                return nlohmann::json(std::string(yyjson_get_str(v),
+                                                  yyjson_get_len(v)));
+            default: {
+                // Array or object: the filter needs the structure (CONTAINS, or a
+                // nested compare), so materialise just this subtree.
+                size_t len = 0;
+                char* raw = yyjson_val_write(v, 0, &len);
+                if (!raw) return std::nullopt;
+                std::optional<nlohmann::json> out;
+                try {
+                    out = nlohmann::json::parse(std::string_view(raw, len));
+                } catch (const nlohmann::json::exception&) {
+                    out = std::nullopt;
+                }
+                free(raw);
+                return out;
+            }
+        }
+    };
+
+    // Document metadata lives at the top level of the stored object, under the
+    // names Document::toJson() writes.
+    if (field == "_id")         return to_json(yyjson_obj_get(root, "id"));
+    if (field == "_created_at") return to_json(yyjson_obj_get(root, "createdAt"));
+    if (field == "_updated_at") return to_json(yyjson_obj_get(root, "updatedAt"));
+    if (field == "_version")    return to_json(yyjson_obj_get(root, "version"));
+
+    yyjson_val* data = yyjson_obj_get(root, "data");
+    if (!data) return std::nullopt;
+
+    if (field.find('.') == std::string::npos) {
+        return to_json(yyjson_obj_get(data, field.c_str()));
+    }
+    // Dotted path: descend objects only, matching getJsonPath's semantics
+    // (arrays are not indexed into).
+    yyjson_val* cur = data;
+    size_t pos = 0;
+    while (pos <= field.size()) {
+        const size_t dot = field.find('.', pos);
+        const std::string seg =
+            field.substr(pos, dot == std::string::npos ? std::string::npos : dot - pos);
+        if (!cur || !yyjson_is_obj(cur)) return std::nullopt;
+        cur = yyjson_obj_get(cur, seg.c_str());
+        if (dot == std::string::npos) break;
+        pos = dot + 1;
+    }
+    return to_json(cur);
+}
+
 void sort_documents(std::vector<smartbotic::database::Document>& docs,
                     const smartbotic::database::Sort& sort) {
     std::sort(docs.begin(), docs.end(),
@@ -442,7 +522,109 @@ ScanResult LmdbDocumentStore::scan(std::string_view collection,
     }
     // -------------------------------------------------------------------
 
+    // -------------------------------------------------------------------
+    // v2.8.0 — two passes: decide cheaply, materialise only what is returned.
+    //
+    // This used to build a full Document for EVERY row just to ask whether it
+    // matched. Over 10,124 real rows (193 MB) that was 3168ms against 369ms for
+    // the yyjson parse alone, so a filtered query took ~2.5s to return ten
+    // documents. Views always add their baked-in filters, so every view query
+    // paid it.
+    //
+    // Pass 1 parses each row with yyjson and evaluates the predicate through a
+    // field resolver, keeping only the ids that match (plus the sort key when
+    // sorting). Pass 2 decodes just the page. Sorting still needs a key for every
+    // match, but a key is one value rather than a whole document.
+    //
+    // A SEARCH filter scans every string in a document, so it cannot be answered
+    // from a per-field resolver; those queries keep the old row-at-a-time path.
+    // -------------------------------------------------------------------
+    const bool wholeDoc =
+        smartbotic::db::storage::filter_eval::needsWholeDocument(query.filters);
+    const bool sorting = query.sort && !query.sort->field.empty();
+
     std::vector<smartbotic::database::Document> matches;
+
+    if (!wholeDoc) {
+        struct Hit {
+            std::string id;
+            nlohmann::json key;      // sort key; null when not sorting
+            bool hasKey = false;
+        };
+        std::vector<Hit> hits;
+
+        int rc2 = mdb_cursor_get(cursor, &k, &v, MDB_FIRST);
+        while (rc2 == MDB_SUCCESS) {
+            if (!is_identity_key(to_sv(k))) {
+                const auto bytes = to_sv(v);
+                yyjson_doc* ydoc = yyjson_read(bytes.data(), bytes.size(), 0);
+                if (ydoc) {
+                    yyjson_val* root = yyjson_doc_get_root(ydoc);
+                    const bool ok =
+                        smartbotic::db::storage::filter_eval::matchesFiltersResolved(
+                            [root](const std::string& f) {
+                                return resolve_from_yyjson(root, f);
+                            },
+                            query.filters);
+                    if (ok) {
+                        Hit h;
+                        h.id = std::string(to_sv(k));
+                        if (sorting) {
+                            auto kv = resolve_from_yyjson(root, query.sort->field);
+                            if (kv) { h.key = *kv; h.hasKey = true; }
+                        }
+                        hits.push_back(std::move(h));
+                    }
+                    yyjson_doc_free(ydoc);
+                }
+                // A row that will not parse cannot be matched; skipping it is the
+                // same outcome the old path reached by throwing on decode, minus
+                // failing the whole query for one bad row.
+            }
+            rc2 = mdb_cursor_get(cursor, &k, &v, MDB_NEXT);
+        }
+        if (rc2 != MDB_NOTFOUND && rc2 != MDB_SUCCESS) {
+            throw_mdb(rc2, "cursor_get");
+        }
+
+        if (sorting) {
+            const bool desc = query.sort->descending;
+            std::stable_sort(hits.begin(), hits.end(),
+                [desc](const Hit& a, const Hit& b) {
+                    // Same ordering rules as sort_documents: missing values sort
+                    // last ascending, and ties break on id for a stable page.
+                    if (!a.hasKey && !b.hasKey) return desc ? a.id > b.id : a.id < b.id;
+                    if (!a.hasKey) return desc;
+                    if (!b.hasKey) return !desc;
+                    if (a.key == b.key) return desc ? a.id > b.id : a.id < b.id;
+                    const bool less = a.key < b.key;
+                    return desc ? !less : less;
+                });
+        }
+
+        result.total_matched = hits.size();
+        const uint64_t start = std::min<uint64_t>(query.offset, hits.size());
+        const uint64_t end = std::min<uint64_t>(
+            static_cast<uint64_t>(query.offset) + query.limit, hits.size());
+        result.documents.reserve(end > start ? end - start : 0);
+        for (uint64_t i = start; i < end; ++i) {
+            MDB_val hk = to_val(hits[i].id);
+            MDB_val hv{0, nullptr};
+            if (mdb_get(rtxn.raw(), *dbi_opt, &hk, &hv) != MDB_SUCCESS) continue;
+            result.documents.push_back(decode_document(to_sv(hv)));
+        }
+        result.has_more = end < result.total_matched;
+
+        if (!query.projection.empty()) {
+            for (auto& d : result.documents) {
+                nlohmann::json data = d.data();
+                apply_projection_inplace(data, query.projection);
+                d.set_data(data);
+            }
+        }
+        return result;
+    }
+
     int rc = mdb_cursor_get(cursor, &k, &v, MDB_FIRST);
     while (rc == MDB_SUCCESS) {
         // Skip the identity sentinel — it is not JSON and would throw in

+ 60 - 8
service/src/storage/filter_eval.hpp

@@ -17,6 +17,7 @@
 
 #include <algorithm>
 #include <cctype>
+#include <functional>
 #include <optional>
 #include <regex>
 #include <sstream>
@@ -129,13 +130,38 @@ resolveFilterValue(const smartbotic::database::Document& doc,
     return getJsonPath(doc.data(), field);
 }
 
-// AND-evaluate every filter in `filters` against `doc`. Returns true iff
-// every filter matches; an empty filter list matches.
-inline bool matchesFilters(const smartbotic::database::Document& doc,
-                           const std::vector<smartbotic::database::Filter>& filters) {
+// True if any filter needs the whole document rather than one field. SEARCH
+// scans every string in the document, so it cannot be answered from a
+// per-field resolver and the caller must materialise the document.
+inline bool needsWholeDocument(
+    const std::vector<smartbotic::database::Filter>& filters) {
+    for (const auto& f : filters) {
+        if (f.op == smartbotic::database::FilterOp::SEARCH) return true;
+    }
+    return false;
+}
+
+// v2.8.0 — AND-evaluate filters using a caller-supplied per-field resolver.
+//
+// This exists so a scan can decide whether a row matches WITHOUT building a
+// Document for it. Materialising every row was 88% of the cost of a filtered
+// query: over 10,124 real rows (193 MB), yyjson parse alone was 369ms while
+// parse + Document::fromJson was 3168ms. A filtered query on that collection took
+// ~2.5s to return ten documents.
+//
+// The comparison logic lives here ONCE and both entry points share it, which is
+// the same reason filter evaluation was lifted out of MemoryStore and
+// document_store_lmdb in v2.0: two private copies drift, and then a divergence
+// between substrates is a code difference rather than a data difference.
+//
+// `resolve` returns nullopt for an absent field. Callers with SEARCH filters must
+// use the Document overload - see needsWholeDocument().
+inline bool matchesFiltersResolved(
+    const std::function<std::optional<nlohmann::json>(const std::string&)>& resolve,
+    const std::vector<smartbotic::database::Filter>& filters) {
     using smartbotic::database::FilterOp;
     for (const auto& filter : filters) {
-        std::optional<nlohmann::json> value = resolveFilterValue(doc, filter.field);
+        std::optional<nlohmann::json> value = resolve(filter.field);
         switch (filter.op) {
             case FilterOp::EXISTS:
                 if (value.has_value() != filter.value.get<bool>()) return false;
@@ -177,12 +203,38 @@ inline bool matchesFilters(const smartbotic::database::Document& doc,
                 }
                 break;
             case FilterOp::SEARCH:
-                if (!filter.value.is_string()) return false;
-                if (!matchesSearch(doc, filter.value.get<std::string>())) return false;
-                break;
+                // Unreachable through this entry point: a SEARCH filter needs the
+                // whole document, so callers must route it to the Document
+                // overload. Fail closed rather than silently matching.
+                return false;
         }
     }
     return true;
 }
 
+
+// AND-evaluate every filter in `filters` against `doc`. Returns true iff
+// every filter matches; an empty filter list matches.
+//
+// Thin wrapper over matchesFiltersResolved so there is one copy of the
+// comparison logic, plus the SEARCH case which needs the whole document.
+inline bool matchesFilters(const smartbotic::database::Document& doc,
+                           const std::vector<smartbotic::database::Filter>& filters) {
+    using smartbotic::database::FilterOp;
+    for (const auto& filter : filters) {
+        if (filter.op == FilterOp::SEARCH) {
+            if (!filter.value.is_string()) return false;
+            if (!matchesSearch(doc, filter.value.get<std::string>())) return false;
+        }
+    }
+    std::vector<smartbotic::database::Filter> rest;
+    rest.reserve(filters.size());
+    for (const auto& f : filters) {
+        if (f.op != FilterOp::SEARCH) rest.push_back(f);
+    }
+    return matchesFiltersResolved(
+        [&doc](const std::string& field) { return resolveFilterValue(doc, field); },
+        rest);
+}
+
 }  // namespace smartbotic::db::storage::filter_eval

+ 127 - 0
tests/test_subdb_identity.cpp

@@ -15,6 +15,7 @@
 #include <filesystem>
 #include <iostream>
 #include <cstdio>
+#include <algorithm>
 #include <set>
 #include <string>
 #include <unistd.h>
@@ -417,6 +418,131 @@ void test_aborted_write_does_not_poison_the_collection() {
     check(store.get("poisoned", "good").has_value(), "get() works after the abort");
 }
 
+// v2.8.0 — the two-pass filtered scan must agree with the old row-at-a-time path
+// on every operator, not just the common ones.
+//
+// scan() now evaluates predicates against a yyjson tree via a field resolver and
+// materialises only the returned page, because building a Document per row was
+// 88% of a filtered query's cost (3168ms vs 369ms for the parse alone over 193 MB
+// of real rows). A resolver that mishandles one operator returns silently wrong
+// data, so this walks the matrix.
+void test_filtered_scan_operator_matrix() {
+    TmpEnv t("filtermatrix");
+    LmdbDocumentStore store(t.env);
+
+    auto put = [&](const std::string& id, const nlohmann::json& data) {
+        Document d;
+        d.id = id;
+        d.collection = "m";
+        d.version = 3;
+        d.createdAt = 1000;
+        d.updatedAt = 2000;
+        d.set_data(data);
+        store.put("m", id, d);
+    };
+
+    put("a", {{"n", 1}, {"kind", "odd"},  {"tags", {"x", "y"}}, {"nest", {{"deep", "hit"}}}});
+    put("b", {{"n", 2}, {"kind", "even"}, {"tags", {"y"}},      {"nest", {{"deep", "miss"}}}});
+    put("c", {{"n", 3}, {"kind", "odd"},  {"tags", nlohmann::json::array()}});
+    put("d", {{"n", 4}, {"kind", "even"}, {"extra", "present"}});
+
+    auto ids = [&](const smartbotic::database::Query& q) {
+        std::vector<std::string> out;
+        for (const auto& d : store.scan("m", q).documents) out.push_back(d.id);
+        std::sort(out.begin(), out.end());
+        return out;
+    };
+    auto q1 = [&](const char* field, smartbotic::database::FilterOp op,
+                  const nlohmann::json& val) {
+        smartbotic::database::Query q;
+        q.limit = 100;
+        smartbotic::database::Filter f;
+        f.field = field; f.op = op; f.value = val;
+        q.filters.push_back(f);
+        return q;
+    };
+    using Op = smartbotic::database::FilterOp;
+
+    check(ids(q1("kind", Op::EQ, "odd")) == (std::vector<std::string>{"a", "c"}),
+          "EQ on a data field");
+    check(ids(q1("kind", Op::NE, "odd")) == (std::vector<std::string>{"b", "d"}),
+          "NE on a data field");
+    check(ids(q1("n", Op::GT, 2)) == (std::vector<std::string>{"c", "d"}), "GT numeric");
+    check(ids(q1("n", Op::GTE, 3)) == (std::vector<std::string>{"c", "d"}), "GTE numeric");
+    check(ids(q1("n", Op::LT, 2)) == (std::vector<std::string>{"a"}), "LT numeric");
+    check(ids(q1("n", Op::LTE, 2)) == (std::vector<std::string>{"a", "b"}), "LTE numeric");
+    check(ids(q1("n", Op::IN, nlohmann::json::array({1, 4}))) ==
+              (std::vector<std::string>{"a", "d"}), "IN");
+    check(ids(q1("tags", Op::CONTAINS, "x")) == (std::vector<std::string>{"a"}),
+          "CONTAINS descends into an array value");
+    check(ids(q1("extra", Op::EXISTS, true)) == (std::vector<std::string>{"d"}),
+          "EXISTS true");
+    check(ids(q1("extra", Op::EXISTS, false)) ==
+              (std::vector<std::string>{"a", "b", "c"}), "EXISTS false");
+    check(ids(q1("kind", Op::REGEX, "^od")) == (std::vector<std::string>{"a", "c"}),
+          "REGEX");
+    check(ids(q1("nest.deep", Op::EQ, "hit")) == (std::vector<std::string>{"a"}),
+          "dotted path descends into data");
+    check(ids(q1("nest.missing", Op::EXISTS, true)).empty(),
+          "a dotted path that does not resolve matches nothing");
+
+    // Document metadata, which lives at the top level of the stored JSON rather
+    // than inside "data".
+    check(ids(q1("_id", Op::EQ, "b")) == (std::vector<std::string>{"b"}), "_id");
+    check(ids(q1("_version", Op::EQ, 3)).size() == 4, "_version");
+    check(ids(q1("_created_at", Op::GTE, 1000)).size() == 4, "_created_at");
+    check(ids(q1("_updated_at", Op::LT, 2000)).empty(), "_updated_at");
+
+    // SEARCH must still work - it needs the whole document, so it takes the old
+    // path.
+    check(ids(q1("", Op::SEARCH, "present")) == (std::vector<std::string>{"d"}),
+          "SEARCH still matches (routed to the whole-document path)");
+    check(ids(q1("", Op::SEARCH, "nothinghere")).empty(), "SEARCH non-match");
+
+    // Sorting, pagination and total_matched over a filtered set.
+    {
+        smartbotic::database::Query q;
+        q.limit = 1;
+        smartbotic::database::Filter f;
+        f.field = "kind"; f.op = Op::EQ; f.value = "odd";
+        q.filters.push_back(f);
+        q.sort = smartbotic::database::Sort{"n", true};   // descending
+        auto page0 = store.scan("m", q);
+        check(page0.total_matched == 2, "total_matched counts matches, not rows");
+        check(page0.documents.size() == 1, "limit honoured");
+        check(page0.documents[0].id == "c", "descending sort picks the highest first");
+        check(page0.has_more, "has_more true mid-set");
+
+        q.offset = 1;
+        auto page1 = store.scan("m", q);
+        check(page1.documents.size() == 1 && page1.documents[0].id == "a",
+              "second page continues the sort order");
+        check(!page1.has_more, "has_more false on the last page");
+
+        q.offset = 5;
+        check(store.scan("m", q).documents.empty(), "offset past the end is empty");
+    }
+
+    // Ascending, and a sort field that is missing from some documents.
+    {
+        smartbotic::database::Query q;
+        q.limit = 10;
+        q.sort = smartbotic::database::Sort{"extra", false};
+        auto res = store.scan("m", q);
+        check(res.total_matched == 4, "no filter plus a sort still totals every row");
+        check(res.documents.size() == 4, "and returns them all");
+        // sort_documents returns `descending` when the LEFT value is missing, so
+        // ascending puts documents that HAVE the field first and the ones missing
+        // it last. The two-pass path copies that rule rather than inventing one.
+        check(res.documents.front().id == "d",
+              "ascending: the document that has the sort field comes first");
+        std::vector<std::string> tail;
+        for (size_t i = 1; i < res.documents.size(); ++i) tail.push_back(res.documents[i].id);
+        check(tail == (std::vector<std::string>{"a", "b", "c"}),
+              "and the ones missing it follow, tie-broken by id");
+    }
+}
+
 }  // namespace
 
 int main() {
@@ -430,6 +556,7 @@ int main() {
     test_scan_limit_zero_reports_total();
     test_scan_fast_path_matches_general_path();
     test_aborted_write_does_not_poison_the_collection();
+    test_filtered_scan_operator_matrix();
 
     std::cout << "passed: " << g_pass << ", failed: " << g_fail << "\n";
     return g_fail == 0 ? 0 : 1;

Некоторые файлы не были показаны из-за большого количества измененных файлов