Obviously support for serials as C++ data types has crept into ddl2cpp and sqlpp23 with the following two pieces of code
and
|
if col_def.cpp_type and col_def.cpp_type.endswith("sqlpp::serial"): |
IMHO this is wrong.
Firstly, all the serial pseudotypes (BIGSERIAL, SMALLSERIAL, SERIAL, SERIAL2, SERIAL4, SERIAL8) are PostgreSQL-specific extension which PostgreSQL documentation doesn't call real types. From the PostgreSQL documentation:
The data types serial and bigserial are not true types, but merely a notational convenience for setting up unique identifier columns (similar to the AUTO_INCREMENT property supported by some other databases). In the current implementation, specifying
CREATE TABLE tablename (
colname SERIAL
);
is equivalent to specifying:
CREATE SEQUENCE tablename_colname_seq;
CREATE TABLE tablename (
colname integer DEFAULT nextval('tablename_colname_seq') NOT NULL
);
So these are not real types even for PostgreSQL, which the moment it sees a "serial" column, essentially turns it into an integer, non-nullable column with a default, auto-increment value.
Secondly, sqlpp23 doesn't have a proper C++ data type to represent a "serial" column. Making sqlpp::serial an alias of sqlpp::integral is wrong. Instead "serial" columns should be treated as integral, non-nullable columns with a default values, just like they have been treated so far.
Thirdly, recognizing a column annotation cpp_type: sqlpp::serial is plain wrong, because there should be no sqlpp::serial in the first place. If someone wants that badly to be able to express the "serial" property through annotations, then there are two possible approaches.
- Add support for expressing the nullable and has_default properties through annotations too. Then instead of using the wrong
-- cpp_type: sqlpp::serial
ustom_type mycol,
one could write the more correct
-- cpp_type: sqlpp::integral, nullable: true, has_default: true
custom_type mycol,
- Or add a special shorthand annotation which combines the three properties into one:
-- serial
custom_type mycol,
The two approaches could be combined, one could add all three annotation properties nullable, has_default and serial (as a shorthand annotation). Just don't add sqlpp::serial as an alias of sqlpp::integral, because it is wrong and misleading.
Obviously support for serials as C++ data types has crept into ddl2cpp and sqlpp23 with the following two pieces of code
sqlpp23/include/sqlpp23/core/type_traits/data_type.h
Line 80 in 474971d
and
sqlpp23/scripts/sqlpp23-ddl2cpp
Line 418 in 474971d
IMHO this is wrong.
Firstly, all the serial pseudotypes (BIGSERIAL, SMALLSERIAL, SERIAL, SERIAL2, SERIAL4, SERIAL8) are PostgreSQL-specific extension which PostgreSQL documentation doesn't call real types. From the PostgreSQL documentation:
So these are not real types even for PostgreSQL, which the moment it sees a "serial" column, essentially turns it into an integer, non-nullable column with a default, auto-increment value.
Secondly, sqlpp23 doesn't have a proper C++ data type to represent a "serial" column. Making
sqlpp::serialan alias ofsqlpp::integralis wrong. Instead "serial" columns should be treated as integral, non-nullable columns with a default values, just like they have been treated so far.Thirdly, recognizing a column annotation
cpp_type: sqlpp::serialis plain wrong, because there should be nosqlpp::serialin the first place. If someone wants that badly to be able to express the "serial" property through annotations, then there are two possible approaches.one could write the more correct
The two approaches could be combined, one could add all three annotation properties
nullable,has_defaultandserial(as a shorthand annotation). Just don't addsqlpp::serialas an alias ofsqlpp::integral, because it is wrong and misleading.