Skip to content

Unregistered/misspelled record types silently yield an empty Reflection #23

Description

@jkalias

Problem

GetRecordFromTypeId (src/reflection.cc:36) looks up the record with operator[]:

auto& meta_struct = instance.records[type_id];
return meta_struct;

std::map::operator[] default-inserts an empty Reflection when the key is absent, so an unregistered or misspelled type returns a record with an empty name and no members instead of signalling an error.

Impact

Operating on an unknown type produces nonsense SQL (empty table name, no columns) that fails downstream with an opaque message, instead of a clear "type not registered" error. This is also inconsistent with Database::GetRecord (src/database.cc), which uses records.at(type_id) and throws on a miss.

Correction — GetRecordFromTypeId is dual-use; do NOT simply make it throw

The original suggested direction ("use .at() in GetRecordFromTypeId") is incomplete and would break registration. That function is called on TWO paths:

  1. Lookup (the buggy path) — QueryPredicate constructor, include/query_predicates.h:83:
    auto record = GetRecordFromTypeId(typeid(T).name()); — here a miss should FAIL.
  2. Create-on-registration — the generated Register<REFLECTABLE>() in include/reflection.h:198
    relies on operator[]'s default-insert to CREATE the entry before populating it:
    auto isRecordRegisterd = instance.records.find(type_id) != instance.records.end();
    if (!isRecordRegisterd) {
        auto& reflectable = GetRecordFromTypeId(type_id);   // must create the entry
        reflectable.name = name;
        ... DEFINE_MEMBER ...
    }
    If GetRecordFromTypeId throws on miss, every registration throws and nothing works.

Corrected direction (two parts)

  • Give registration its own explicit create path instead of leaning on GetRecordFromTypeId's
    side effect — e.g. in Register<REFLECTABLE>() use the in-scope instance.records[type_id]
    directly (the surrounding find guard already scopes the create-on-miss to registration).
  • Then make GetRecordFromTypeId a pure lookup that fails fast: find + throw a clear
    std::runtime_error naming the type when absent (<stdexcept> in src/reflection.cc).

After that, the only remaining caller of GetRecordFromTypeId is the query/lookup path, so an
unregistered type raises a clear error at the point of use. Confirm there are no other callers that
depend on the create-on-miss behavior (currently only Database::GetRecord looks up records, and it
already uses .at()).

Suggested direction (superseded — see Correction above)

Make the lookup consistent and fail-fast: use .at() (or an explicit find + throw) in
GetRecordFromTypeId.
Correct in spirit, but must be paired with moving registration off that
function first, per the Correction section.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions