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

[Volker Krause]

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

Ship it!


Looks good to me. Let's discuss the TODO once you are in the office ;)

- Volker


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/