-
-
src/Lunr/DataAccess/DMLQueryBuilderInterface.php (Diff revision 3) If it only were that easy :) See the usual flow of the defining a query would be like this: $builder->select('foo')->from('bar')->where('bat'); $builder2->select('bar')->from('foo)->where('bat')->union($builder->get_select_query()); Also consider that you can do more than one union. Having to use a different query builder for every union clause is a bit heavy.
-
Change Summary:
The union implementation is completely changed in comparison the previous patches. For the parentheses I used a function called query_value(), introduced also in my other patch for conditional functionality (http://reviews.lunr.nl/r/48/).
Change Summary:
One function compound() for UNION, EXCEPT and INTERSECT Removed the escaping from the union function. Boolean operator for UNION instead of string Fixes documentation.
Diff: |
Revision 5 (+151 -3) |
---|
-
-
src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 5) they don't really need to be kept separate. WHERE HAVING and ON do because they each go in a different part of the query, but UNION, EXCEPT and INTERSECT are all "query connectors", so they all go to the same place.
-
src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 5) that's not quite it. Like this you get: SELECT * FROM table UNION (SELECT * FROM table) whereas we want: (SELECT * FROM table) UNION (SELECT * FROM table)
-
src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 5) that's union specific so would put it into the union method.
Change Summary:
Fixes the issue mentioned by Heinz in the previews review.
Diff: |
Revision 6 (+165 -2) |
---|
-
-
src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revisions 5 - 6) IMHO this is the wrong way around. Compounds are an extension to a normal query, so you want to handle the standard case first, and if we do have compounds continue on.
-
src/Lunr/DataAccess/MySQLDMLQueryBuilder.php (Diff revisions 5 - 6) Code wise it's fine, but the naming is less than ideal. If you think yourself as a developer using the API, what does it mean if I set operator = TRUE on a UNION? Will it make it a TRUE UNION or what? If you name it $all it's clearer IMHO
Change Summary:
some code adjustments, based on the comments.
Diff: |
Revision 7 (+415 -286)
|
---|
-
-
src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revisions 6 - 7) This works, but is not very future friendly :) $standard = 'SELECT ' . $this->implode_query($components); if ($this->compound == '') { return $standard; } $components = array(); $components[] = 'compound'; return $standard . ' ' . implode_query($components);
Change Summary:
Adds the proposed changes, by Heinz, in the query construction. See the last comments of the last review.
Diff: |
Revision 8 (+420 -286)
|
---|
Change Summary:
The same patch without the changes of the latest commit of Leonidas "Core: Upgraded C2DM class to GCM."
Diff: |
Revision 9 (+174 -732)
|
---|
Change Summary:
The same patch as in review #8. Not able to remove the latest commit. Changes of the 4 first changed files are already pushed in master.
Diff: |
Revision 10 (+420 -286)
|
---|
-
-
src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 10) here you are initializing the components array twice. are you sure you want that?
-
src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 10) here you are initializing the components array twice. are you sure you want that?
-
-
src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 10) I suppose this should be $this->compound now
-
-
src/Lunr/DataAccess/MySQLDMLQueryBuilder.php (Diff revision 10) hmm, nitpick. IMHO the spacing should happen in sql_compound(). All you define here is the keyword to use, which should not really have surrounding spaces yet.
-
src/Lunr/DataAccess/Tests/DatabaseDMLQueryBuilderQueryPartsTest.php (Diff revision 10) We don't really need separate test methods for all of these. You can just create a testCompoundQuery() method and connect it with a dataProvider.
Change Summary:
changes according to the comments of the last review
Branch: |
|
||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 11 (+129 -8)
|