[Kde-pim] Review Request: refactor INNER JOIN in QueryBuilder

[Milian Wolff]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3907/#review5502
-----------------------------------------------------------



/trunk/kdesupport/akonadi/server/src/storage/datastore.cpp
<http://reviewboard.kde.org/r/3907/#comment5186>

    note how this Join condition was missing before



/trunk/kdesupport/akonadi/server/src/storage/entities-source.xsl
<http://reviewboard.kde.org/r/3907/#comment5185>

    SelectQueryBuilder does that automagically now



/trunk/kdesupport/akonadi/server/src/storage/itemqueryhelper.cpp
<http://reviewboard.kde.org/r/3907/#comment5184>

    not required for the WHERE below



/trunk/kdesupport/akonadi/server/src/storage/itemretriever.cpp
<http://reviewboard.kde.org/r/3907/#comment5183>

    InnerJoin again like before, only the PartTable join must be LeftJoined



/trunk/kdesupport/akonadi/server/src/storage/query.h
<http://reviewboard.kde.org/r/3907/#comment5182>

    why is this required to be implemented for the QList<>::contains() in the QueryBuilder::addJoin ? Shouldn't the Compiler generate that for me since none of the members is a pointer?



/trunk/kdesupport/akonadi/server/src/storage/query.cpp
<http://reviewboard.kde.org/r/3907/#comment5181>

    was randomly initialized before leading to wrong operator== results



/trunk/kdesupport/akonadi/server/src/storage/querybuilder.cpp
<http://reviewboard.kde.org/r/3907/#comment5180>

    sqlite cannot cope with joined UPDATE queries, this never worked - needs to be fixed with subqueries



/trunk/kdesupport/akonadi/server/tests/unittest/querybuildertest.cpp
<http://reviewboard.kde.org/r/3907/#comment5179>

    since QueryBuilder hsa a required QSTring arg in it's ctor now it cannot be declared as a MetaType - major PITA :(


- Milian


On 2010-05-07 17:21:52, Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3907/
> -----------------------------------------------------------
> 
> (Updated 2010-05-07 17:21:52)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> refactor handling of INNER JOINs in the QueryBuilder
> 
> Now the QueryBuilder takes it's main table in the Ctor as it's required for all queries anyways.
> All joins, even INNER JOINs are handled via the new addJoin() method that takes a JoinType argument.
> 
> - LEFT JOIN is only supported for SELECT queries
> - INNER JOIN is only supported for SELECT and UPDATE queries
> 
> This change to the handling of INNER JOINs is required, since the ',' as shortcut for an INNER JOIN
> cannot be mixed with LEFT JOINs, while the explicit version can be. See also:
> 
> http://dev.mysql.com/doc/refman/5.0/en/join.html
> 
> The part about 'unknown column' when using ',' for INNER JOINs.
> 
> TODO:
> 
> How should multiple joins with different conditions / different type be handled?
> Maybe we should add a getter so that the ItemHelper methods can see which tables are already
> joined (no matter how) so they can only add their WHERE condition without
> caring about joins.
> 
> 
> Diffs
> -----
> 
>   /trunk/kdesupport/akonadi/server/src/cachecleaner.cpp 1123998 
>   /trunk/kdesupport/akonadi/server/src/handler/expunge.cpp 1123998 
>   /trunk/kdesupport/akonadi/server/src/handler/fetchhelper.cpp 1123998 
>   /trunk/kdesupport/akonadi/server/src/handler/list.cpp 1123998 
>   /trunk/kdesupport/akonadi/server/src/handlerhelper.cpp 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/countquerybuilder.h 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/datastore.cpp 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/entities-source.xsl 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/entity.h 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/itemqueryhelper.cpp 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/itemretriever.cpp 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/query.h 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/query.cpp 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/querybuilder.h 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/querybuilder.cpp 1123998 
>   /trunk/kdesupport/akonadi/server/src/storage/selectquerybuilder.h 1123998 
>   /trunk/kdesupport/akonadi/server/tests/unittest/querybuildertest.h 1123998 
>   /trunk/kdesupport/akonadi/server/tests/unittest/querybuildertest.cpp 1123998 
> 
> Diff: http://reviewboard.kde.org/r/3907/diff
> 
> 
> Testing
> -------
> 
> Ran most of the unit tests and they pass. Searching should now also work again without any assertion, but of course there is still https://bugs.kde.org/show_bug.cgi?id=223812 .
> 
> 
> Thanks,
> 
> Milian
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/