simolus3
Manager v2
This PR includes the following changes to the manager filter API.
Remove Value Filter for Custom Type
Until now the manage would create 2 filters for a column that has a TypeConverter. One that accepts the Custom Type and another that uses the raw value. To avoid polluting the namespace of filterset, the ability to filter on a raw value will be removed. It is extremely uncommon that a filter on this raw data would be performed, so I think it's worth it. E.G
f.category.status(Status.active)
f.category.statusValue(7) // Removed
References don't use a callback
This will help with readability:
items.filter((f)=>f.category.name("Foo"))
instead of
items.filter((f)=>f.category((f)=>f.name("Foo")))
Remove Id filter for references
Until now, references columns would create 2 columns, one for the actual column, and one for the reference object. We will remove the "Id" filter. Before:
items.filter((f)=>f.categoryId(5))
After:
items.filter((f)=>f.category.id(5))
This looks like we are performing a join for no reason, but we aren't! Under the hood no join is performed, a filter is applied to the actual category column!
Other Changes
- Docs for extensions have been removed until the internal manager API is more consistent. We don't want people writing code at this lower level.
- Mark lower level classes as internal, but still expose them for the generated code. This will change once the lower level APIs are more stable.
- Disable manager generation on older dart 2 code
@simolus3 I tried debugging this myself, but I'm at a loss.
There are Book Clubs and Members. I'm filtering book clubs with 3 members. This SQL query returns 1 club, which is CORRECT!
SELECT
"book_club"."id" AS "book_club.id",
"book_club"."name" AS "book_club.name"
FROM
"book_club"
LEFT OUTER JOIN "person" "book_club__id__person__club" ON "book_club"."id" = "book_club__id__person__club"."club"
GROUP BY
"book_club"."_rowid_"
HAVING
COUNT(*) = ?;
HOWEVER, COUNTING DOESN'T!
SELECT
DISTINCT COUNT(*) AS "c0"
FROM
"book_club"
LEFT OUTER JOIN "person" "book_club__id__person__club" ON "book_club"."id" = "book_club__id__person__club"."club"
GROUP BY
"book_club"."_rowid_"
HAVING
COUNT(*) = ?;
This will always return whatever the number filter was, in this case, it's returning 3! There are only 2 clubs in the database!
Even accessing the raw value, gets {c0: 3}
This happens even without DISTINCT
This is either a really bad bug in drift or I'm doing something wrong.
This is the test that's failing, It's mind boggling.
https://github.com/dickermoshe/drift/blob/21b7607a77bfeb50b8cb56119043a636d10fa74d/drift/test/manager/manager_referenced_filter_test.dart#L161-L173
This is how the django ORM does counts. Can this be created using drift?
SELECT
COUNT(*)
FROM
(
SELECT
"api_product"."sku" AS "col1"
FROM
"api_product"
LEFT OUTER JOIN "api_listing" ON ("api_product"."sku" = "api_listing"."product_id")
GROUP BY
1
HAVING
COUNT("api_listing"."id") = 3
)
@simolus3 Figured it out! As usual your awesome documentation made this easy!
@simolus3
Counting can only really be done if there is a rowid or only 1 primary key.
Even ORMs that have been developed for the past 20 years don't support a database with more than 1 primary key.
We can either throw an error if they try to count (it's like that now) or ignore such tables when generating manager (makes more sense, less shoot-foot-gun issues).
What should be done?
To support Postgres and MariaDB, where we don't have a rowid column, I think we should strive to support compound primary keys as well.
Counting can only really be done if there is a rowid or only 1 primary key.
To be sure I'm understanding this correctly, the problem is that we care about the amount of unique rows of one table in a select statement that may have additional joins?
That's not really obvious from the documentation of that method, which just claims to return the amount of rows that would be returned by the whole statement. If that's the number we actually care about, what's wrong with SELECT count(*) FROM (the actual statement that would otherwise be generated)?
If we need "amount of unique rows from table x appearing in a joined result set", couldn't we add all the primary key columns to the inner result set and then do a sum + count combination? E.g. if the primary key is the columns foo, bar, baz:
SELECT sum(*) FROM (SELECT count(*) FROM (inner query) GROUP BY foo, bar, baz)
Then the inner query gives us how many rows there are for each unique combination of primary key columns and the final sum gives us the total amount. But I also don't see how that information is relevant in most queries.
That's real smart.
On a side note, I was looking at other ORM designs and have decided that aggregates should be done in another method and filtering should focus on filtering.
// Class where aggregates are predefined, these will be added to the filterset
class CategoryAggregateModel extends ...{
...
final itemCount = ...;
...
}
category.annotate(CategoryAggregateModel()).filter((f)=>f.itemCount(5))
This will reduce code duplication if we ever decide to add a way to return the aggregated values.
I'm going to revert back to this design for filtering reverse references
// Category with todo with id of 5
category.filter(f.todos((f)=>f.id(5))) // Remove .exist() or .any()
Aggregates will be for a later release, they add complexity to the point where my limited knowledge of SQL is starting to show.
I've followed your guidance on counting and have begun adding more and more tests, It's starting to come together nicely.
Nice! Let me know if there's anything ready for me to take a more in-depth look at :)
@simolus3 The refactoring here is quite signifying. LMK if there is anything I can do to make your reviewing on this PR easier.
This PR is complete