-
-
Notifications
You must be signed in to change notification settings - Fork 342
Correcting ordinary insert statement for tables with composite primary keys #1468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The if-condition used for filtering primary key columns must be the same at run-time as at compile-time.
... to the mapped object type, avoiding hard to read error messages later.
Up until this commit, the functions were referred to as "composite PK functions", although this actually always meant "table PK functions", regardless of whether they were single or composite primary keys defined at the table level.
* Improved understanding through legibility.
For proper type safety `storage_t<>::insert()` needs to return an `sqlite3_int64` instead of an `int`. This is the best way to correct the problem. Usually, compilers just accept narrowing type conversions, the nicest thing that can happen is a warning message, the worst is an error depending on compiler flags. The programmer has to handle the rest and it is the best thing to do anyway.
* Check column definition up front when making the column. * Renamed internal traits. * Removed unit test file `is_column_with_insertable_primary_key.cpp`. It became superfluous during the course of development in the past years. Column constraints are checked elsewhere already, and it is now only a duplicate of `is_primary_key_insertable.cpp`.
SQLite documentation clearly states that an auto-incrementable primary key column must be a 64-bit signed integer type.
337084c to
93a23fe
Compare
This approach keeps the previous way if all composite primary key columns have a default value, yet still enables inserting records if the table has composite primary key columns with only some or no default values.
* WITHOUT ROWID is only available since SQLite 3.8.2. * FTS5: consider that it can be explicitly enabled with `SQLITE_ORM_ENABLE_FTS5`. * Auxiliary virtual table columns are only available since SQLite 3.24.0. * Hidden virtual table columns have no version attached.
728b390 to
4007b1a
Compare
Only types that can represent a 64-bit signed integer - either integral or custom types - have the required properties to be an alias for the rowid and can cover the whole value range of it.
4007b1a to
0886bd5
Compare
Removes the deprecation notice for using non-64-bit integral types as rowid aliases, deferring the type size integrity to the programmer. There are valid concerns that deprecating integral types with less precision than `sqlite3_int64` will provoke situations where people will start complaining about many compiler warnings. Note: I have retained the partial template specializations for validating integral types for better understanding.
There are valid concerns that deprecating integral types with less precision than `sqlite3_int64` will lead to people complaining about many warnings. As the ordinary insert routine is considered a bit like 'legacy' and is subject to multi-threading issues as well the decision is to implement another object insertion routine in the future and just leave things as they are.
dev/storage.h
Outdated
| using without_rowid = typename table_type::is_without_rowid; | ||
|
|
||
| table.template for_each_column_excluding< /// | ||
| mpl::disjunction< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: probably this could be more readable with local aliases like
using mpl_or = mpl::disjunction;
using mpl_and = mpl::conjunction;
using mpl_not = mpl::not_;There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, if this is a serious wish, then we should flatten the namespaces and just define everything in terms of mpl_ prefixes...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not so serious wish, it is just a nit, cause you're more familiar with mpl part of the lib
Improvements:
Other improvements:
make_table().getquery statement.Fixes