mindplay-dk / sql

Database framework and query builder
Other
18 stars 6 forks source link

Minor design issues with Table / Column models #46

Open mindplay-dk opened 5 years ago

mindplay-dk commented 5 years ago

The Table::requiredColumn() factory-method accepts a $default value, but this is never used.

The InsertQuery builder is currently the only query-builder that uses the Column::getDefault() and Column::isRequired() methods - and the default value is only applied when the column is not required.

So the $default argument to Table::requiredColumn() probably should be removed.

^ fixed in master

There is a minor design issue regarding the concept of "optional" and "required" - as it's described in the documentation at the moment:

Note that "required" and "optional" do not necessarily correlate 1:1 with IS NULL in your schema. For example, a column could be "required" but still allow SQL NULL values - in this case, "required" means you must explicitly supply a null-value e.g. to the INSERT query-builder, which may be safer and more explicit for some use-cases.

While this is probably accurate (given how the required and default properties are used, as described above) the concept of nullable isn't currently described by the Column model.

Given that all types (per the description of Type) must support nulls, nullable isn't supported by the Type model either.

Currently, an INSERT or UPDATE query-builder will accept a null-value for any column, and attempts to build the query - which then fails at the database-level.

We should probably address that?