Coder Social home page Coder Social logo

Comments (27)

dickermoshe avatar dickermoshe commented on July 18, 2024 2

I agree with everything above. Thank you for taking the time to read my whole post!

I spent a while fighting tooth and nail with darts type system, but I think I came up with a flexible type-safe manager that looks and feels great.

I'll get back to you later tonight with the details.

Thank you so much!

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024 1

@simolus3
I started working on a PR, but before I continue, I would love to get an answer if this is something you would want to merge with into this repo.

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024 1

If you want to try it out, see here https://github.com/dickermoshe/drift
Just do this if you just want to install it https://drift.simonbinder.eu/docs/internals/

There is an added manager on the generated database that has all the helpers on them.

You're docs were so easy to use btw, great job!

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024 1

I added modular support, although the generator is 🍝 code now. Needs to be cleaned up.
I'm going to see if I missed anything for the manager and then start writing docstrings.
I opened a Draft PR. I'll ping you there when I need your input.

from drift.

simolus3 avatar simolus3 commented on July 18, 2024

Sorry for the slow response, and thank you for starting to work on this!

Generally, I think drift shouldn't hide away the underlying relational structure. We should make it much easier to use, but never try to create the illusion it's not there. In the past, I've considered fluent APIs that also automatically resolve joins for relationships to be out of scope for this package (even though drift should be a great building block for other packages providing these features). I think a fluent API for single tables like you've suggested doesn't really obscure anything though, so I think this is something that can be part of drift.
One concern I have is the increase in code size required for this, but even if we just have a builder option to disable generating this for users who don't need it, it should be fine.

So yes, contributions in this area would be welcome and I'd be happy to merge this into the drift package.

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

No worries about the delayed response! I totally understand.

Just to double check - are joins acceptable to include in this fluent API?

todoItems.filter().ageBetween(1, 2).and.userFilter((user)=>user.nameEqualTo("Bob"))

If yes, what's your opinion on what to name backlinks?

from drift.

simolus3 avatar simolus3 commented on July 18, 2024

Just to double check - are joins acceptable to include in this fluent API?

Would the example you've posted include both the todo item and the user in its result? Or is it just joining users to write more expressive filters for todo items?
I think it's problematic if selecting from todoItems alone added an implicit join to user for results - these things should be explicit. With something like userFilter, I think the explicit reference makes this clear.

If yes, what's your opinion on what to name backlinks?

I'm not sure what you mean with this or what the problem is, could you explain that a bit more? (sorry for the naive question, I haven't used a full ORM in a long time)

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

Or is it just joining users to write more expressive filters for todo items?

Yes. Nothing magical is happening.
This would only return Todo items, nothing extra is being returned

I'm not sure what you mean with this or what the problem is.

An example

class TodoItems extends Table {
  // ...
  IntColumn get category =>
      integer().nullable().references(TodoCategories, #id)();
}

@DataClassName("Category")
class TodoCategories extends Table {
  IntColumn get id => integer().autoIncrement()();
  // and more columns...
}

If we wanted to query TodoItems that exist in a category 5 we could do the following:

final category = category.getById(5)
category.todoItemsSet.filter()...
        ^^^^^^^^^^^^^^

However, a name needs to be made up.

I'll link the docs from isar:
Isar uses this. https://isar.dev/links.html#backlinks

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

@simolus3

I've been trying to keep everything concise to avoid polluting the codebase.
This isn't a replacement or abstraction of the SQL layer, but rather an intuitive alternative to help developers write basic queries easily, attracting a wider audience without extensive SQL experience.

The idea is to introduce a manager that can do basic stuff:

1. Top level functions for common tasks

Adding these top-level functions to a manager increase readability significantly.
This is mostly due to the fact that a SQL query starts with the action (select/into) instead of a query.
Those used to writing raw SQL may find this self-explanatory, however the following is a more readable equivalent.

Proposed Syntax:

db.select(db.todoItems).watch()
db.select(db.todoItems).get()
db.delete(db.todoItems).do()
db.into(db.todoItems).insert(item)

 ||
_||_
\  /
 \/

manager.todoItems.watchAll()
manager.todoItems.getAll()
manager.todoItems.deleteAll()
manager.todoItems.insert(item)

2. Simple Query Building

The current expression builder is sick! Being able to use & and | to build queries is great.
However there are 2 issues I've found that make using it for simple tasks burdensome.

  1. SQL terms for SQL queries
    select(todoItems)..where((tbl) => tbl.name.isBetweenValues("Bob", "Jack")
    'Between a String, what does that mean?', may ask a developer with less SQL experience.

  2. A very polluted auto-complete in the IDE:
    These will all show up when building a query using the current where method.
    a. Expressions that take other expressions.
    b. Expressions that produce expressions which aren't Boolean.
    c. Other TableInfo,Table and GeneratedColumn attributes.
    Table Auto Complete:

image

Column Auto Complete:
image

Using monads, this looks much cleaner, with much better auto-complete

Proposed Syntax:

select(todoItems)..where((tbl) => tbl.age.isBetweenValues(2, 5) & tbl.name.equals("Bob"))

 ||
_||_
\  /
 \/

manager.todoItems.filter.ageBetween(2,5).and.nameIs('Bob')

3. Filter on Related Values

I we only wanted todos with a specific category attribute, it would be pretty messy.

Proposed Syntax:

class TodoItems extends Table {
  IntColumn get category =>
      integer().nullable().references(TodoCategories, #id)();
}
@DataClassName("Category")
class TodoCategories extends Table {
  IntColumn get id => integer().autoIncrement()();
}

manager.todoItems.filter.categoryFilter((c)=>c.nameIs("Work")).get()

Note:

  • This won't remove any part of existing syntax.
  • Will be completely optional.
  • Migrations won't be able to use this.
  • Will only generate for tables, not for views.
  • Only add milliseconds to code generation.
  • No breaking changes

I've implemented most of the API and have started work on implementing with the builder.
I'm trying to keep this as light as possible, a full ORM would belong in it's own package. I think this is a perfect in between.
Lmk what you think.

from drift.

simolus3 avatar simolus3 commented on July 18, 2024

If we wanted to query TodoItems that exist in a category 5 we could do the following:

If I understand it correctly that doesn't need a backlink, right? We could do todoItems.filter().categoryFilter((category)=>category.idEqualTo(5))? We need them for queries like "give me all categories containing a todo item with a title containing Important". More generally, it seems like we need them for things we'd use EXISTS queries in SQL, so maybe the name could start with something like has? E.g. for the schema you've posted

category.hasTodoItem((item) => item.titleContains('Important'))
  1. Top level functions for common tasks

I fully agree with all of this - I've tried to make these easier with extensions on tables, but I agree that having them in a central place leads to better discoverability and makes the package easier to use.

  1. Simple Query Building

I also agree with this. Just one question: Do we support nested AND/ORs? I think something like (a OR B) AND c might be hard to express in the linear sequence, right? I'm fine if this is something not supported because it's arguably not something you need everyday, but I think it would be great if there was some sort of escape hatch giving full access to the existing query builder within the new API. For instance, something like manager.todoItems.filter.ageBetween(2,5).and.sqlPredicate((tbl) => ...), giving full acces to the table and allowing more complex expressions if needed.

  1. Filter on Related Values

The syntax looks fine to me, and it can be implemented with an implicit join or with a subquery.

Will only generate for tables, not for views.

Just curious, what's the problem with generating this for views too? Or do we need the full insert/update functionality for every entity in the manager?

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

@simolus3

Seems that drift can't use a join on a delete statement.
How should I go about this?
Execute a select to get ids and then run delete?
Use a subquery?
Something else?

This is what I've done for now:

final deleteStatement = db.delete(table)
        ..where(
            (_) => _table.rowId.isInQuery(selectOnlyRowIdStatement));

Also, can rowId be used reliably as the a primary key?

from drift.

simolus3 avatar simolus3 commented on July 18, 2024

I agree with the subquery approach for now 👍 We should have support for joins on updates and deletes, I'll take a look.

Also, can rowId be used reliably as the a primary key?

I think it's fine for now. Technically it's not correct because:

  1. There are WITHOUT ROWID tables for which this doesn't work.
  2. It also doesn't work outside of sqlite3 (we should support PostgreSQL and MariaDB as well where possible).

A sound approach would be to look up the table's primary key and then use that as a filter. So selectOnlyRowIdStatement should select the entire matching primary key, and the where predicate would then filter on every column in the primary key. But we can keep the current behavior for now, I'll look into adding joins for updates and deletes and then we can use those instead of adding more complexity to the temporary workaround.

from drift.

simolus3 avatar simolus3 commented on July 18, 2024

Oh hold on, deletes can't even have joins (at least not in sqlite3).

So I think we'll have to use the "subquery selecting a primary key" approach. You can use TableInfo.$primaryKey to find the primary key of a table. If the primary key was (a, b, c), I think the SQL we could generate is something like:

DELETE FROM table outer
  WHERE EXISTS (SELECT * FROM table inner JOIN ... WHERE $userCondition AND inner.a = outer.a AND inner.b = outer.b AND inner.c = outer.c)

You can make drift generate tables with aliases by using the alias function, EXISTS subqueries can be written with existsQuery. Unfortunately it looks like there's no simpler way for deletes that's still reliable (or at least I can't come up with anything).

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

@simolus3
I've finished what is arguably the hardest part of all this.
You can now create infinity nested ordering and filters, with aliased joins created on the fly.

For example:

db.managers.todoEntries
            .filter((f) =>
                f.categoryFilter(
                    (f) => (f.color.equals(Colors.black12) | f.name("Bob"))) &
                f.dueDate.isAfter(DateTime.now()))
            .orderBy(
                (o) => o.dueDate.asc() & o.categoryOrderBy((o) => o.name.asc()))
            .get()

The above will return all todos whose category is due today and either have a Colors.black12 color or name is "Bob"
It will then return the result ordered by dueDate, and then by the category id.

This took hours days to make, I hope you like it.

The code that I've added to drift is really neat and well documented. Doing this while managing joins with aliases added much more complexity to the code that I would have liked, but I think I've documented it pretty nicely

drift_dev is another story, the code is not as neat as I would have liked. I don't know how the code generator you wrote works, it seems really complex. I will need guidance to write the builder in a neater manner, as well as how to add options for toggling manager generation.

Now only fetching data works, I will be adding delete and update, as well as create.

When I have CRUD working and it's all documented I will open a PR.

At that point I will need guidance on what tests should be created.

from drift.

Mike278 avatar Mike278 commented on July 18, 2024

Doing this while managing joins with aliases

Idk if it'd make it easier, but I'd be curious if the API can be implemented in terms of subqueries rather than joins. The API only affects how many rows are returned, not how many columns, so a join shouldn't be completely necessary. Subqueries are generally easier for SQLite to optimize, and you might be able to avoid some aliasing since subqueries don't introduce any new columns to the result set like joins do.

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

Damn, I researched it for like a second, seems that subqueries are slower, so made it with joins.
I hope it's actually that much quicker, because it was lots of work to get done right 😂

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

@simolus3
I started adding support for filtering accross backlinks. and for aggregates

Example

Query all categories that have more than 5 todo entries.

categories.filter((f) => f.referencedTodoEntries((f) => f.id.count.isBiggerThan(5)))

Request For Comment

How should we name the backlinks filter (referencedTodoEntries).
We can't just use the backlinked table name, there could be duplicates.
E.G

class TodoEntries extends Table with AutoIncrementingPrimaryKey {
  IntColumn get category => integer().nullable().references(Categories, #id)(); // referencedTodoEntries
  IntColumn get oldCategory => integer().nullable().references(Categories, #id)(); // referencedTodoEntries
}

class Categories extends Table with AutoIncrementingPrimaryKey {
  /// ...
}

To make it unique it could be constructed from the table name and column name:

  • referencedTodoEntriesCategory
  • referencedTodoEntriesOldCategory

But this looks really nasty.

Potential Solutions

How about we add a optional position argument to references

class TodoEntries extends Table with AutoIncrementingPrimaryKey {
  IntColumn get category => integer().nullable().references(Categories, #id,"currentEntries")(); // currentEntries
  IntColumn get oldCategory => integer().nullable().references(Categories, #id,"oldEntries")(); // oldEntries
}

If it is not specified we construct one base on the table name and column name above.

If you feel this isnt ideal, then what about adding another annoatation @BackLink("currentEntries") ?

Disclamer

I understand that modifying this section of the codebase is extremly sensitive. Let me know what you are most comfortable with

from drift.

simolus3 avatar simolus3 commented on July 18, 2024

This looks really cool! I didn't have a chance to play around with it yet, I'll take a deeper look tomorrow.

I think the DSL itself should only be concerned with things that actually end up in the database schema, so I don't think we should put a name in the references clause directly. I'm fine with a @BackLink annotation for this purpose though.
We should consider using the short name by default if there's only one reference between the two tables. That avoids the whole problem in most cases, a downside is that adding a second reference would then break the existing names.

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

In Django when there are back refs that clash, it will refuse to run untill they are set.
I think we should automaticy do todoItemRefs, using just the table name. If there are 2 columns from a single table, then we don't generate ANY backlinks and display a warning:

The Category table is referenced by the TodoItem table more than once...Please use BackLink to specify the name...

That way there isnt any breaking code if they add columns.

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

I've added create & bulk_create to the manager.

db.managers.textEntries.create((o) => o(description: "Eat Cheese"))
db.managers.textEntries.bulkCreate((o) => [o(description: "More!"), o(description: "Way More!")])

I'm working on update now.

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

I'm trying to leave all the logic in the drift package so that the generated code is smaller.
It's a nightmare dealing with generics, but It's a fun challenge.

from drift.

simolus3 avatar simolus3 commented on July 18, 2024

Awesome work! And yeah, I've seen the generics :D I wonder if we can unify the WithOrdering and WithFiltering markers into a single type since implementing both interfaces and then exposing them as necessary? They don't seem to have a table-specific interface at the moment. We can also replace the database class generic with a non-generic GeneratedDatabase field I think (which is required for things like modular generation where we don't know the database for a table).

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

I don't want users applying filters or orderings twice.
So once a filter is applied it returns a manager without that capability.
The same goes for ordering.
This means that filter is not the same in both classes.
We could make .filter().filter() allowed, and just say that it's an and

We can also replace the database class generic with a non-generic GeneratedDatabase field I think (which is required for things like modular generation where we don't know the database for a table).

I don't understand, The class needs to know this.

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

I wonder if we can unify the WithOrdering and WithFiltering markers into a single type since implementing both interfaces and then exposing them as necessary?

I pushed an update that simplifies it. dart pub get and rebuild.

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

We can also replace the database class generic with a non-generic GeneratedDatabase field I think (which is required for things like modular generation where we don't know the database for a table).

I don't think the manager will work fully with modular generation. The getters for filtering on foreign references use db.tableName

Example

class $TodoEntriesTableFilterComposer
    extends FilterComposer<_$AppDatabase, $TodoEntriesTable> {
  $TodoEntriesTableFilterComposer(super.db, super.table);
  ComposableFilter categoryRef(
      ComposableFilter Function($CategoriesTableFilterComposer f) f) {
    return referenced(
        referencedTable: state.db.categories, // <<<<<<<<<<<<<<<<<<<<<<<<<<< HERE 😭
        getCurrentColumn: (f) => f.category,
        getReferencedColumn: (f) => f.id,
        getReferencedComposer: (db, table) =>
            $CategoriesTableFilterComposer(db, table),
        builder: f);
  }
}

If we were unable to pass a typed database, then these foreign filters wouldn't be able to be generated :(

What option can I use to detect if code generation is being done modularly?

from drift.

simolus3 avatar simolus3 commented on July 18, 2024

I don't want users applying filters or orderings twice.

Perhaps I didn't check this correctly. I agree that $TableFilterComposer and $TableOrderingComposer should be separate classes. But last I checked there were WithOrdering and WithFiltering classes that looked identical - it seems like you've already removed them in the meantime though.

What option can I use to detect if code generation is being done modularly?

In the generator, you can use scope.generationOptions.isModular. You can then use this method to find tables:

/// Find a result set by its [name] in the database. The result is cached.
T resultSet<T extends ResultSetImplementation>(String name) {
return _cache.knownEntities[name]! as T;
}

So you'd call it like state.db.resultSet<$CategoriesTable>('categories'). Another difference with modular code generation is that the manager classes should probably also be split across different files (you can see an example for that build mode in examples/modular. The idea is to generate files separately for each input file defining a table instead of bundling everything in the file for the database). But it's fine to focus on the monolithic build mode for now, I can help with the port to modular generation afterwards.

from drift.

dickermoshe avatar dickermoshe commented on July 18, 2024

Great!, I'll use that all over!

from drift.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.