Skip to content
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

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

Merged
merged 2 commits into from
Apr 21, 2024

Conversation

groue
Copy link
Owner

@groue groue commented Apr 20, 2024

It was reported in #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
Copy link
Owner Author

groue commented Apr 20, 2024

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

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

@groue groue force-pushed the dev/singleton-rewrite branch 3 times, most recently from c55d42c to 52c5aeb Compare April 21, 2024 10:21
@groue groue merged commit 3f3cfa5 into development Apr 21, 2024
21 checks passed
@groue groue deleted the dev/singleton-rewrite branch April 21, 2024 13:10
@DamienPetrilli
Copy link

@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
Copy link
Owner Author

groue commented Apr 21, 2024

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants