Fresh eyed perspective and a mish-mash of thoughts

[jackd]

So having seen #3461 yesterday I started putting some ideas down... then realized it was too long for that issue and split it into multiple issues... then realized those issues were all kind of inter-related so have smooshed them all back together again... then I iterated a couple more times and now I've written a monster. I apologies in advance for the length.

I also apologise in advance if some of these come across a little blunt. Some are very opinionated - and I openly admit those opinions come from a place of limited experience with both code generation and running large/long term projects. Others are a little petty. Feel free to take what you want and ignore everything else. I'm happy to go into more detail on any that aren't sufficiently well fleshed out or contribute to their implementation if desired. Also happy to split things up again into separate issues for discussion if there's interest.

Here's hoping it's taken in the spirit it's intended :).

A data-independent query/statement construction layer

Drift treats queries very differently to tables and views. This makes a lot of sense since tables and views are persistent, while queries aren't. That said, I feel there's a missing layer between persistent entities (tables, view, triggers) and the procedural operations performed by DAOs and databases that are difficult or impossible to run in raw SQL.

Specifically, I think drift is missing a mechanism to construct static/parameterized sql statements that are independent of the records in the actual database through the dart API. I would like to see an interface similar to View that allows construction of queries and arbitrary sql statements based on schema information alone, either at compile time or dynamically. DAOs and databases would then connect these to executors and add an additional layer on top of this for procedural logic.

I've experimented a little with the generated code from .drift files and this is close to what I think should be possible from dart. Even then though I'd like to be able to generate this modularly without introducing a DAO.

Taking an example from the docs:

await db.users.insertOne(UsersCompanion(
  id: Value.absent(), 
  username: Value('user'), 
));

await (db.update(db.users)..where((tbl) => tbl.id.equals(1)))
    .write(UsersCompanion(username: Value("Updated name")));

Conceptually, this is just two sql statements being constructed in dart. I think it would be much cleaner / more managable to be able to do:

final op1 = users.insertOne(UsersCompanion(
  id: Value.absent(), 
  username: Value('user'), 
));
final op2 = (update(users)..where((tbl) => tbl.id.equals(1)))
    .write(UsersCompanion(username: Value("Updated name")));
await db.run(compoundOp(op1, op2));

Here I use op to mean a dart structure that represents an sql statement, and compoundOp just creates a single operation out of multiple sub-operations. Running an op would just be a matter of forming the sql string and passing it to the executor. While I think the biggest appeal of this is separation of concerns / project maintainability/understandability, I feel there would have to be performance benefits too with only 1 FFI call.

Parameter-free generated DAO classes

There comes a point in every project where things get big and messy and you want to rip things apart. Currently generated DAOs are parameterized by the database that will use them. This doesn't play nicely with modularity. If I have a database that uses a DAO, the DAO shouldn't need to know about the database. I believe this can be resolved by making a generated DAO also generate an interface class that gives access to the relevant TableInfos and an associated manager (with corresponding abstract interface), and the database using it can implement that database.

As an example, in the documentation we have:

@DriftAccessor(tables: [Todos])
class TodosDao extends DatabaseAccessor<MyDatabase> with _$TodosDaoMixin {
  // this constructor is required so that the main database can create an instance
  // of this object.
  TodosDao(MyDatabase db) : super(db);

  // queries using `db` and `todos`
}

I would rather it be used as

@DriftAccessor(tables: [Todos])
class TodosDao extends $TodosDao {

  // queries using `db` and `todos`

}

where

// generated code
abstract interface class $TodosDaoManager {
  $$TodosTableTableManager get todos;
}

abstract interface class $TodosDaoDatabase {
  TodosInfo get todos;
  $TodosDaoManager get manager;
}

class $TodosDao extends DatabaseAccessor<$TodosDaoDatabase> {
  // generated methods using inherited $TodoDaosDatabase db;
}

and then databases using the DAO will have generated classes:

class $MyDatabaseManager implements $TodoDaosManager {
  // otherwise generated as usual
}
class $MyDatabase extends GeneratedDatabase implements $TodosDaoDatabase {
  // otherwise generated as usual
  final todosDao = TodosDao(this);
}

Another step would be to make $TodosDaoDatabase a concrete class and NOT make $MyDatabase implement it, but rather have $MyDatabase construct a $TodosDaoDatabase as a private member of it's tables. That way MyDatabase doesn't explicitly use a table used in TodosDaos it doesn't need to be part of $MyDatabase's public interface.

Only 1 of Table / TableInfo

It's taken me a while to understand the difference between Table and TableInfo. I'm still not entirely sure I understand the purpose of the distinction, but based on my current understanding I think they're highly counter-intuitive. A Table is a HasResultSet, which makes me think it has a result set - something I might be able to get with, I don't know, a get method maybe. Wrong. TableInfo on the other hand is a HasResultSetImplementation. If anything I would think a HasResultSet would be a subclass of HasResultSetImplementation - you need an implementation to get something right? Also wrong. That said, HasResultSetImplementation does have a get method that gets... what I would call a result set.

Intuitively, I think of a Table as something with data in it, and if I had to guess, TableInfo would contain metadata about that table - e.g. column names and types. That's exactly the opposite of what they are. Tables represent schema definitions, while TableInfo contains the schema info as well as methods to access the records stored in the database.

Remove symbols for references

Symbols irk me. Having just argued to get rid of 1 of either Table or TableInfo, I'm now going to argue in favor of keeping them both - albeit in different forms. Let's say we keep TableInfo much that same. What if we defined our member tables like:

@DataClassName('Category')
class Categories extends Table {
    static const id = Column<int>();
    static const name = Column<String>();
}

class Todos extends Table {
  static const id = Column<int>();

  @References(Categories.id)
  static const categoryId = Column<int>();
}

These classes would never be instantiated - enforced by either a private constructor on Table or some other means - and serve purely to provide schema information and serialization/deserialization.

Do we need to copy code?

Having to put in exports just so generated code can work seems wrong. json_serialization handles custom serialization and default values with annotations - is there places that approach can't be used in drift?

Companions -> Builder

Ok, this is admittedly pretty small/very much personal preference, but as someone who's been playing around with dart on and off for a while, I'm familiar with builders since built_value days. It's not nearly so popular now thanks to freezed, but I still know what a builder does and why it's needed. Maybe it's popular in some circles I've never been a part of, but I've never heard of a Companion while programming. It's not hard to read the documentation to understand it, but it's IMO unintuitive, and I don't think it looks great wrapping everything in Value. I'm not suggesting it would need to be built on top of built_value - though if that kills multiple birds with one stone all the better.

If we consider the example from the docs:

await db.users.insertOne(UsersCompanion(
  id: Value.absent(), 
  username: Value('user'), 
));

await (db.update(db.users)..where((tbl) => tbl.id.equals(1)))
    .write(UsersCompanion(username: Value("Updated name")));

I would much rather use

await db.users.insertOne((b) => b..username = 'user');

await (db.update(db.users)..where((tbl) => tbl.id.equals(1)))
    .write((b) => b..username = 'Updated name');

Here, b would be a UsersBuilder something like the following:

class UsersBuilder {
    int? _id;
    bool _idIsSet = false; // might have to be @protected depending on how it gets integrated
    int? get id => _id; // arguably this wouldn't be necessary for the above use case
    set id(int? value) {
        _idIsSet = true;
        _id = value;
    }
    // same for username
}

Consider renaming Selectable

Another arguably petty one. I know naming is hard, and there's probably a very good reason this was originally used - but if I can't select a Selectable then I would argue at least one of those is named incorrectly.

An incomplete idea: records as data classes

If you've made it this far, I'll leave you with an idea that I'm a long way from done with yet, but could potentially radically reduce the amount of code generated.

Could records be used as the base data class, and other functionality be implemented as extensions on templated records similar to Future.wait? For the moment let's forget about elegant APIs and focus on type safety: then let's suppose we have something like the following:

abstract class SqlSerializer<S, D> {
    S toSql(D dartData);
    D fromSql(S sqlData);
}

/// Haven't thought through this interface all that much yet...
class Column<S, D> implements SqlSerializer<S, D> {
    final String name;
    const Column(this.name);
}

/// A few basic implementations
class TrivialColumn<T> implements Column<T, T> {
    T toSql(T dartData) => dartData;
    T fromSql(T sqlData) => sqlData;
}

typedef IntColumn = TrivialColumn<int>;
typedef StringColumn = TrivialColumn<String>;
class DateTimeColumn implements Column<int, DateTime> {
    // probably got those method names wrong...
    DateTime fromSql(int sqlData) => DateTime.sinceEpoch(sqlData);
    int toSql(DateTime dartData) => dartData.millisecondsSinceEpoch();
}

abstract class TableInfo<S, D> implements SqlSerializer<S, D>{
    List<Column<dynamic>> get allColumns;
}

const todosColumns = (IntColumn('id'), TextColumn('title'), DateTimeColumn('createdAt'), IntColumn('categoryId'));
final todos = todosColumn.toTable();

const categoriesColumns = (IntColumn('id'), TextColumn('name'));
final categories = todoColumns.toTable();

This is supported with drift library extension methods:

abstract class TableInfo<C, S, D> {
    C get structuredColumns;
    List<Column<dynamic>> get allColumns;


}

class TableInfo1<S1, D1> implements TableInfo<(S1,), (D1,), (Column<S1, D1>)> {
    final (Column<S1, D1>) structuredColumns;
    List<Column<dynamic>> get allColumns => structuredColumns.toList();

    const TableInfo1(this.structuredColumns);
    
    Column<S1, D1> get $1 => structuredColumns.$1;

    (S1,) toSql((D1,) dartData) => ($1.toSql(dartData.$1),);
    (D1,) fromSql((S1,) sqlData) => ($1.fromSql(sqlData.$1),);
}

extension ColumnRecord1<S1, D1> on (Column<S1, D1>) {
    TableInfo1<S1, D1> toTable() => TableInfo1(this);
    List<Column<dynamic>> toList() => [$1];
} 

class TableInfo2<S1, S2, D1, D2> implements TableInfo<(S1, S2), (D1, D2)> {
    final (Column<S1, D1>, Column(S2, D2)) structuredColumns;
    List<Column<dynamic>> get allColumns => structuredColumns.toList();
    const TableInfo1(this.structuredColumns);

    Column<T1> get $1 => structuredColumns.$1;
    Column<T2> get $2 => structuredColumns.$2;

    (S1, S2) toSql((D1, D2,) dartData) => ($1.toSql(dartData.$1), $2.toSql(dartData.$2));
    (D1, D2,) fromSql((S1, S2) sqlData) => ($1.fromSql(sqlData.$1), $2.fromSql(sqlData.$2));
}

extension ColumnRecord2<S1, S2, D1, D2> on (Column<S1, D1>, Column<S2, D2>) {
    TableInfo2<S1> toTable() => TableInfo2(this);
    List<Column<dynamic>> toList() => [$1, $2];
} 

// etc for length 3 records, length 4 records, ...
// it would have to be a fixed number, but that number could be big

We could then have

final todosCategoryView = viewAs(
    selection: (todos, categories) => (todos.$1, todos.$2, categories.$2),
    from: todos.join(
        categories, 
        onColumns: (todos, categories) => todos.$3.equals(categories.$1),
    ),
);

Obviously there's a lot of details that would need to be investigated further, but I believe most of the simple stuff could be handled like this in a type safe way without any project-specific code generation (generating ColumnRecordNs would probably be wise - I nearly tore my hair out just writing the N=2 case - but that would be part of drift, not for client libraries).

Now I'll be the first to admit those $1s and $2s are a lot less graceful than named fields. Unfortunately records with different named fields are different types, so we can't just extend over all possible record names like we can positional-only fields. My gut feeling is we could get essentially everything we want by generating code to handle record structuring (positional-only records -> named only records and back again) - i.e. have as much implementation on the flattened record implementations in the drift library and just transform to a named record interface at the API surface. I don't know much about dart internals but I feel those destructuring/restructuring operations would have to be pretty cheap, and I'm pretty sure each could be generated in 2 lines of code.

[simolus3]

I really appreciate these discussions, thank you for sharing your ideas! This is exactly what I wanted to provoke when publishing the v3 wishlist :)

A data-independent query/statement construction layer

I've experimented a bit with the approach of keeping the query builder and query executors / database classes separated (essentially having a standalone query builder library). I like the freedom that this unlocks, but it also feels like something that could become an inconvenience for most users.

Today, Selectables can be run directly by calling get() and the other methods, so they are bound to some database. When keeping them separate, you'd have to use something like db.run(query) or db.watch(query). Admittedly, that's not too different from the current API which also involves db at some point, but typically at the beginning. From an API standpoint, db.select(users).get() is more discoverable though than db.run(select(users)). Auto-complete makes the first one one easier to type. I've considered still keeping this separate query builder around and then introducing wrapper classes (this can also be done quite nicely with extension types) so that db.select(users) is still possible, but that still is a lot of duplicate effort.

If I have a database that uses a DAO, the DAO shouldn't need to know about the database

Fully agreed, I've added this to the todo list. FWIW, I also generally think that we should be moving into a more modular direction. But it's very hard to do without augmentations. With them, we could just generate code wherever a table is defined and introduce an augmentation on the table to start queries. Borrowing your first idea:

Users.query().filter((row) => row.id.equals(3)).watch(database)

Something like this could reduce the necessity for DAOs.

Only 1 of Table / TableInfo

Yeah, parts of this mess have also grown out of additional indirections squeezed in to not break existing users. From a conceptional standpoint, the table class you write is the interface (providing getters for columns). Drift generates an implementation that:

1. Represents the resolved structure of the table (so we can get a list of columns to build queries and CREATE TABLE statements).
2. Implements your table interface, so that you can do where((row) => row.id.equals(3)).

But the fact that TableInfo is a mixin is really not helpful. In my current draft for v3, the hierarchy for tables is class $Users extends GeneratedTable<...> implements Users. Your table would still be declared as class Users extends Table (or interface class Users implements Table if you want to be pedantic).
In the DSL layer, I don't really see a use for a common interface between tables and views like the HasResultSet we have today. For the implementation, GeneratedTable extends ResultSet.

Remove symbols for references

The main problem with static table classes is that you can use the same table multiple times in one statement. E.g. you could have

class Points extends Table {
  static const id = Column<int>();
}

class Routes extends Table {
  @References(Points.id)
  static const start = Column<int>();

  @References(Points.id)
  static const end = Column<int>();
}

Today, it is very neat that we can just write:

final startPoint  = alias(points, 'start');
final endPoint = alias(points, 'end');

final query = select(routes).join([
  innerJoin(startPoint, startPoint.id.equalsExp(routes.start)),
  innerJoin(endPoint, endPoint.id.equalsExp(routes.end)),
]);

And then join startPoint and endPoint with Routes, treating them as different tables throughout the query builder. This works really well for building queries and reading results, and it only works because we can instantiate tables like that. I don't really see a solution for that without instantiating tables.

Or is the idea that the generated table classes would provide typed getters for columns then, without inheriting from the definitions? That would make the table definitions kind of heavy for no reason.
FWIW, I've discussed improvements to the way tables are defined with @dickermoshe a while ago, and we also agree that an annotation-based approach can be easier. This depends on augmentations, but something like this similar to Room on Android could be interesting:

// You write an annotated data class, drift generates a table structure
@DriftTable()
@futureDataClassAugmentationMagic
final class Category {
  final int id;
  final String name;
}

@DriftTable()
@futureDataClassAugmentationMagic
final class TodoItem {
  final int id;
  @References(Category.$.id)
  final int categoryId;
}

But my preferred approach is to focus on breaking changes that are mostly internal to drift for now, and then unlock new generation styles as an opt-in builder options once augmentations or other language features enable them.

Do we need to copy code?

Also something that will magically work with augmentations. Until then, I recommend modular code generation which will make drift emit its own libraries with independent imports.
(one could argue that this should be a default for v3, but most users also don't like too many generated files cluttering up their project. So shared parts have an advantage there).

Companions -> Builder

Yeah I stole the companion name from Kotlin, but I agree that it's not exactly an intuitive concept. Maybe it should have been called PartialUser or something.

Here, b would be a UsersBuilder something like the following:

I like that idea - it's kind of similar to what the manager is doing today for inserts (from an API standpoint to avoid explicit value wrappers at least).
I think Builder is not an ideal terminology here because there is no way to go from an Builder to the class it's supposed to build without a database operation. But a mutable class following perhaps the Partial naming scheme with an interface similar to the builder you're suggesting is an interesting idea.

Consider renaming Selectable

Maybe it should be called Query?

An incomplete idea: records as data classes

I agree with using records for data classes, but I'm not sure about using them to define tables. I intend to remove some mostly useless functionality from tables (mostly the verification in Dart, databases do that already) which should reduce their code size significantly. If we also replace the data class with a record, I suspect most of the code (excluding the manager code) will be made up of companions.

It's a neat trick to avoid code generation altogether, but as you've also mentioned we can't get to named records without any codegen and then once we have it, I don't see an advantage over the current definitions.

[jackd]

Apologies for the late response - spent some time prototyping record overloading implementations, and in doing so don't believe they're as great as I thought they could be. During that I was also looking for an interface name for a certain family of objects and went for Selectable... only to realize that's exactly what I'd advocated against. I still think it's wrong, but I understand it now at least - something you can select from, not something you can select. Anyway, regarding other points:

When keeping them separate, you'd have to use something like db.run(query) or db.watch(query)

Seems like a small price to pay for a clearer separation of concerns / type hierarchy, but I won't whip a dead horse.

Something like this could reduce the necessity for DAOs.

My thoughts exactly. I like the idea of namespacing queries, but if that's all they're doing then DAO isn't really the right name. I might even argue that any operations that need to make multiple calls to the sqlite engine don't really belong in the database class itself, but rather should be in a separate layer on top - and preferably delegated to the user rather than in drift.

Or is the idea that the generated table classes would provide typed getters for columns then, without inheriting from the definitions? That would make the table definitions kind of heavy for no reason.

You're correct: to clarify, a static MyTable class that doesn't do much other than give static consts to be referenced, and a MyTableData singleton class with top-level MyTableData myTable instance with same column names but different types/implementations. MyTable.foo would be a static const for references, myTable.foo would be used in query builders. I'm sure a lot of users would not understand the intricacies when they first start, but they don't really need to. I'm sure a lot of flutter users think Theme.of(context) returns a Theme object - but their ignorance doesn't affect their intuitive use of it. Admittedly generating MyTable just for references sounds heavy just to avoid symbols... but I wouldn't say it's for no reason.

@DriftTable()
@futureDataClassAugmentationMagic
final class TodoItem {
  final int id;
  @References(Category.$.id)
  final int categoryId;
}

I don't know exactly how magic augmentations are going to be, but I'm under the impression that it's currently impossible to have compile-time constants of the form Foo.bar.baz, and I'm not aware of anything in the augmentation proposals that will affect that. I could definitely be wrong. Is Category.$ related to the augmentation spec itself, or something the drift would generate using augmentations? If that's not possible, it -would- be possible if we just got rid of the period - Category$.id - with a separate Category$ class with only static members... but that's essentially what I'm proposing with different names :)

I think Builder is not an ideal terminology here because there is no way to go from an Builder to the class it's supposed to build

Absolutely agree. What about FooUpdates? An insert isn't exaclty an update, but maybe a generated interface along the lines of

foo.insert(requiredField1: x, requiredField2: y, updates: (u) => u
  ..optionalField = z
);

update vs partial naming isn't a hill I care enough about to die on though.