Restructure classes for table database objects #1456
Conversation
* Factored out common functionality into common base classes and mixins. * Renamed structures. * Split CTE table class into its own structure, effectively decoupling it from a table DBO.
* Split virtual table facilities into: a module tag, virtual table traits that can be specialized, an intermediary description resulting from a `using module` expression and a concrete virtual table class. * Renamed table DBO classes. * Removed the `_t` suffix from table DBO classes and renamed `virtual_table_t` -> `virtual_table` and `table_t` -> `base_table`, where the latter denotes a table that actually stores data. * Finally replaced the 'without rowid' NTTP by a type parameter.
... instead of when making the definition of the table using a specific module. Additionally: * Split virtual table modules into separate header files.
trueqbit
force-pushed
the
restructure-table-dbos
branch
from
0a799ab to
464cabb
Compare
| * @param lambda Lambda called for each column. Function signature: `void(auto& column)` | ||
| */ | ||
| template<class L> | ||
| void for_each_column(L&& lambda) const { |
There was a problem hiding this comment.
why do we pass an r-value here? if we don't call std::move(lambda) on the next line then it goes further as an l-value. In that case the smarter way is just passing an l-value, probably const l-value
There was a problem hiding this comment.
Well, it's just taking anything, which is usually a lambda in its real sense, i.e. created ad-hoc. In the library's logic there's nNo need to pass it on as an rvalue, so we just pass it on as an lvalue. const-qualifying it is not good as the lambda might be modifyable.
| * | ||
| * [Deprecation notice] This factory function is deprecated and will be removed in v1.11. | ||
| */ | ||
| template<class O, class M, class... Cs> |
There was a problem hiding this comment.
can we add deprecate C++ tag here to produce compiler's warnings?
There was a problem hiding this comment.
I already added the deprecation warning to the using_fts5<O>() factory function. This should be enough, otherwise you get two warnings...
|
|
||
| const auto res = sync_schema_result::already_in_sync; | ||
| context_t context{this->db_objects}; | ||
| const serializer_context<db_objects_type> context{this->db_objects}; |
2 participants
Sharing is caring!
With the potential SQL views as yet another schema object type, code needs to be duplicated - or better factored into shared class templates.
In this PR:
using moduleexpression and a concrete virtual table class._tsuffix. Since the advent of alias template for type aliases IMO it isn't the best choice to use it for the library's regular types.virtual_table_t->virtual_table.table_ttobase_tabledenoting that it is a table that actually stores data. "base table" is the best word in SQL land for this.index_t,trigger_t) if we agree on this different naming convention. Let me know what you think [yes/no/revert].Additionally:
Further changes since the last review on Sep 30 2025:
make_virtual_table<O>()instead of when callingusing_module<O>().dbstattable into an eponymous virtual table, additionally with the option of creating further instances of it.