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

non-nullable attributes should be marked as @required in generated classes #25

Closed
MohiuddinM opened this issue May 15, 2019 · 3 comments

Comments

@MohiuddinM
Copy link

No description provided.

@simolus3
Copy link
Owner

This is due to an unfortunate design choice made by me pretty early on. We can't really make them @required as UpdateStatement.write ignores non-null fields.

This, btw., is required for some queries. Let's say we had a users table with a (required) id and name column and an optional age column. To express UPDATE users SET age = 12 WHERE age IS NULL, we'd use (update(users)..where((u) => isNull(u.age)).write(User(age: 12)). For this, we construct a User object without the id and name fields, although they would be required for other queries. Also, if the id column had an auto_increment, we should be able to write into(users).insert(User(name: 'Test', age: 18) without the analyzer complaining about omitting the id field.

This situation is obviously obviously not ideal and will only get worse when Dart gets null-safe types as we'd have to make each field nullable even though most aren't. At the moment, it's not that bad as the library will enforce that all required fields for a specific query are set. A possible solution might be to generate multiple data classes per table (for reading, updating, and inserting), but I'll have to think about this some more. Other ideas are definitely most welcome.

@simolus3
Copy link
Owner

I think I have a solution that could work. For each data class, we also generate an update companion, which would be used for inserts and updates. It stores whether a field is absent or explicitly set to null, solving this problem and enabling a clean solution for #34.
I'm afraid this would be a major backwards incompatible change and it probably has to wait for a 2.0 version of moor, for which I've planned some other major changes which still need some work.

@simolus3
Copy link
Owner

I think I managed to implement the update companion classes without breaking backwards compatibility on develop, errors should go away after re-running the build step.
You might get warnings about required parameters which aren't set on updates (and possibly on inserts). Let's say you had a Todos table with an auto-increment id and some content (both non-nullable). Now, doing into(todos).insert(TodoEntry(content: 'some content')) will give a warning about the non-nullable id field not being present. You can use a TodosCompanion(content: Value('some content')) instead. Btw. if you wan't to set a field to null during insert, that's not a problem anymore: TodosCompanion(content: Value.absent()). Previously, doing TodoEntry(content: null) would ignore the content entirely. Further, the companion objects can be used with a const constructor.

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

No branches or pull requests

2 participants