groue / GRDB.swift

A toolkit for SQLite databases, with a focus on application development
MIT License
6.61k stars 677 forks source link

Update the "Single-Row Tables" guide, with support for default values #1534

Closed groue closed 2 months ago

groue commented 2 months ago

It was reported in https://github.com/groue/GRDB.swift/discussions/1526 that the Single-Row Tables guide did not deal very well with non-null configuration columns.

This pull request removes the bad advice, and suggests a technique for providing default values for configurations that have no value yet, in a way that reminds UserDefaults.register(defaults:).

groue commented 2 months ago

@DamienPetrilli, maybe you could check the new GRDB/Documentation.docc/SingleRowTables.md?

@yqiang I added a mention of JSON columns as well.

DamienPetrilli commented 2 months ago

@DamienPetrilli, maybe you could check the new GRDB/Documentation.docc/SingleRowTables.md?

@yqiang I added a mention of JSON columns as well.

Very good!Thanks!

I think the only thing which is unclear is what happens when you need to add more settings along the way.

You can't modify the table easily once there is a row. As all fields are mandatory, adding a column will cause the migration to fail.

You could save the existing data and re-insert it, but you are back to: how to deal with missing values?

Knowing the guide do not recommend to inject default values during the migrations.

groue commented 2 months ago

Very good!Thanks!

Thanks!

I think the only thing which is unclear is what happens when you need to add more settings along the way.

You can't modify the table easily once there is a row. As all fields are mandatory, adding a column will cause the migration to fail.

I purposely removed any univocal advice for migrations. As this issue surfaced, thanks to your input, the correct solution depends on the app.

The new guide (raw, still waiting to be published as I write) mentions one advantage of the nullable columns:

As application evolves, application will need to add new configuration columns. It is not always possible to provide a sensible default value for these new columns, at the moment the table is modified. On the other side, it is generally possible to deal with those NULL values at runtime.

I also reminded the reader that they were responsible for their application 😅:

Despite those arguments, some apps absolutely require a value. In this case, don't weaken the application logic and make sure the database can't store a NULL value.

For non-null values, the best way to define them is left as an exercise for the reader. There's always a solution, of course, because we humans are still in the control the machine, lol! But the guide has stopped favoring a particular one. The old guide could create cognitive dissonance in the reader's mind, as exemplified by this issue!


You could save the existing data and re-insert it, but you are back to: how to deal with missing values?

That's what the new guide deals with!

DamienPetrilli commented 2 months ago

That's great! I agree the developers should decide for themselves and the guide should stay neutral.