Add C++ query builder#4664
Conversation
rekhoff
left a comment
There was a problem hiding this comment.
Looks good to me. Some fairly minor nits here.
Co-authored-by: Ryan <r.ekhoff@clockworklabs.io> Signed-off-by: Jason Larabie <jason@clockworklabs.io>
…t.md Co-authored-by: Ryan <r.ekhoff@clockworklabs.io> Signed-off-by: Jason Larabie <jason@clockworklabs.io>
|
Updated the core query builder to carry generated column and index-column metadata alongside each table source. This was needed so downstream codegen can reason about the source shape directly instead of reconstructing it indirectly. In practice, that lets the client generator produce source-aware query authoring APIs, expose the right fields and lookup surfaces, and support a much better Blueprint/C++ developer experience without maintaining a separate query-builder model. |
…or C++ and Unreal
rekhoff
left a comment
There was a problem hiding this comment.
Just one minor nit, otherwise looks good
Co-authored-by: Ryan <r.ekhoff@clockworklabs.io> Signed-off-by: Jason Larabie <jason@clockworklabs.io>
# Conflicts: # crates/bindings-cpp/include/spacetimedb/internal/v10_builder.h
| // `where_ix` based on the predicate signature. | ||
| template<typename TFn> | ||
| [[nodiscard]] auto where(TFn&& predicate) const { | ||
| if constexpr (std::is_invocable_v<TFn, const TCols&, const TIxCols&>) { |
There was a problem hiding this comment.
Why does the where predicate take both TCols and TIxCols? Index columns aren't relevant for where right?
There was a problem hiding this comment.
Index columns aren’t required for where semantically. This split is there so chained queries don’t lose access to indexed-column predicates after earlier query steps. This isn't necessary for the pure C++ query builder, but I'm reusing this on the Unreal C++ side and want to keep them aligned which means a little extra API.
There was a problem hiding this comment.
Is there a way to keep the index cols internal and not part of the user facing api? To maintain consistency with the other query builders?
There was a problem hiding this comment.
Is there a way to keep the index cols internal and not part of the user facing api? To maintain consistency with the other query builders?
Yes and I should have done that! It's been cleaned up now so the where_ix/where_col are no longer in the public api.
There was a problem hiding this comment.
Apologies for the long feedback loop on this one @JasonAtClockwork, but it looks like this still may be part of the public api. Is it currently possible to write an expression like this:
users.where([](const auto& cols, const auto& ix) {
return ix.id.eq(cols.id);
});It really should be:
users.where([](const auto& cols) {
return cols.id.eq(...);
});Because indexed columns are only applicable/necessary for joins.
# Conflicts: # crates/bindings-cpp/include/spacetimedb/internal/v10_builder.h
Description of Changes
API and ABI breaking changes
Expected complexity level and risk
3 - Mostly contained to C++ bindings, but it touches macros, view registration/serialization, and module-def generation, so there are a few places where the pieces need to stay in sync.
Testing
I've done end to end testing of I think every type as well as built some tests to confirm the SQL output.