Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
7 changes: 7 additions & 0 deletions include/query_predicates.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

#include <functional>
#include <memory>
#include <stdexcept>
#include <string>
#include <vector>

Expand Down Expand Up @@ -82,13 +83,19 @@ class REFLECTION_EXPORT QueryPredicate : public QueryPredicateBase {
: symbol_(symbol) {
auto record = GetRecordFromTypeId(typeid(T).name());
auto offset = OffsetFromStart(fn);
auto found = false;
for (auto i = 0; i < record.member_metadata.size(); ++i) {
if (record.member_metadata[i].offset == offset) {
member_name_ = record.member_metadata[i].name;
value_ = value_retrieval((void*)&value, record.member_metadata[i].storage_class);
found = true;
break;
}
}
if (!found) {
throw std::runtime_error("No registered member of '" + record.name +
"' matches the given pointer-to-member (type id: " + typeid(T).name() + ")");
}
}

template <typename T, typename R>
Expand Down
5 changes: 4 additions & 1 deletion include/reflection.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,10 @@ static std::string CAT(Register, REFLECTABLE)() {
ReflectionRegister& instance = *GetReflectionRegisterInstance();
auto isRecordRegisterd = instance.records.find(type_id) != instance.records.end();
if (!isRecordRegisterd) {
auto& reflectable = GetRecordFromTypeId(type_id);
// Create the entry directly (operator[] default-inserts on miss); this is registration's
// own create path, scoped by the find() guard above, and is intentionally not routed
// through GetRecordFromTypeId, which is a pure lookup that throws on a miss
auto& reflectable = instance.records[type_id];
reflectable.name = name;

// store member metadata
Expand Down
8 changes: 6 additions & 2 deletions src/reflection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "reflection.h"

#include <memory>
#include <stdexcept>

static std::unique_ptr<ReflectionRegister> p = nullptr;

Expand All @@ -35,8 +36,11 @@ ReflectionRegister* GetReflectionRegisterInstance() {

Reflection& GetRecordFromTypeId(const std::string& type_id) {
ReflectionRegister& instance = *GetReflectionRegisterInstance();
auto& meta_struct = instance.records[type_id];
return meta_struct;
auto it = instance.records.find(type_id);
if (it == instance.records.end()) {
throw std::runtime_error("Reflection lookup failed: type not registered: " + type_id);
}
return it->second;
}

char* GetMemberAddress(void* precord, const Reflection& record, const size_t i) {
Expand Down
35 changes: 35 additions & 0 deletions tests/query_predicates_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,3 +207,38 @@ TEST(QueryPredicatesTest, LikeInsideAndSurvivesCloneWithEscapeClause) {
EXPECT_EQ(R"(%50\%%)", bindings[0].text_value);
EXPECT_EQ(30, bindings[1].int_value);
}

namespace {
// A plain struct that is deliberately never run through the REFLECTABLE/FIELDS registration
// macros, so its type id never appears in the reflection registry.
struct UnregisteredRecord {
int64_t id;
int64_t value;
};

// A plain struct that is also never run through the registration macros, but is manually and
// incompletely registered below (name only, no member metadata) to exercise the
// registered-but-offset-mismatch guard, as distinct from the unregistered-type guard above.
struct MismatchedRecord {
int64_t id;
int64_t value;
};
} // namespace

TEST(QueryPredicatesTest, PredicateConstructionThrowsForUnregisteredType) {
// #23: GetRecordFromTypeId must fail fast for a type that was never registered, instead of
// std::map::operator[] silently default-inserting an empty Reflection (empty table name, no
// columns), which would otherwise surface later as an opaque SQLite prepare error
EXPECT_THROW(Equal(&UnregisteredRecord::value, 42), std::runtime_error);
}

TEST(QueryPredicatesTest, PredicateConstructionThrowsWhenNoMemberMatches) {
// #22: even for a registered type, if no member_metadata entry's offset matches the
// pointer-to-member (here because the type was registered by hand with no members at all,
// rather than via the FIELDS macro), the QueryPredicate constructor must fail fast instead
// of silently leaving member_name_ empty and emitting malformed SQL like " = ?"
auto& instance = *GetReflectionRegisterInstance();
instance.records[typeid(MismatchedRecord).name()].name = "MismatchedRecord";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Remove the fake record after the expectation

This inserts a permanent record into the process-wide reflection registry. If this test runs before any database-initializing test in the same unit_tests process, for example with --gtest_shuffle or a local filter that runs DB tests afterward, Database::Database iterates every registry entry and CreateTableQuery::PrepareSql() will try to create MismatchedRecord with an empty column list (CREATE TABLE IF NOT EXISTS MismatchedRecord ();), causing initialization to throw. Use RAII cleanup/erase around this expectation so the test does not leak global state.

Useful? React with 👍 / 👎.

@jkalias jkalias Jul 7, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Confirmed and fixed in dcc2d17, with a stronger repro than I expected: running the pre-fix suite with --gtest_shuffle across 5 random seeds failed 4 of them (6-37 cascading failures each) exactly as you described - only the unshuffled default order happened to dodge it. Added a small RAII ScopedRegistryCleanup that erases the hand-inserted MismatchedRecord entry on scope exit; reran 10 shuffled seeds post-fix and all pass.


EXPECT_THROW(Equal(&MismatchedRecord::value, 42), std::runtime_error);
}
Loading