[DataAccess]Add compound query functionality to Lunr 0.2 (UNION, INTERSECT, EXCEPT)

Review Request #52 — Created Feb. 11, 2013 and submitted

dinos
Lunr
https://github.com/dinostheo/lunr
lunr
Adds compound functionality.
Implements UNION.
unit tests
  • 0
  • 0
  • 15
  • 0
  • 15
Description From Last Updated
dinos
dinos
pprkut
  1. 
      
  2. 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.
  3. src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
     
     
    Not our problem.
  4. 
      
dinos
olivier
  1. 
      
  2. src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 4)
     
     
     
     
     
     
     
    And what is the description?
    1. I also noticed it :)
      It comes fixed in the next patch that I am writing right now
  3. 
      
dinos
pprkut
  1. 
      
  2. 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.
  3. 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)
  4. that's union specific so would put it into the union method.
  5. 
      
dinos
pprkut
  1. 
      
  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.
  3. 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
  4. 
      
dinos
pprkut
  1. 
      
  2. 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);
  3. 
      
dinos
dinos
dinos
leo
  1. 
      
  2. src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 10)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    here you are initializing the components array twice. are you sure you want that?
    1. The first time it uses the select components, while in the second only the compound(union, etc).
      It could also have a different name.
    2. Same name is fine. Saves a tiny bit of memory :)
  3. src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 10)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    here you are initializing the components array twice. are you sure you want that?
  4. 
      
pprkut
  1. 
      
  2. src/Lunr/DataAccess/DatabaseDMLQueryBuilder.php (Diff revision 10)
     
     
     
     
    I suppose this should be $this->compound now
  3. 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.
  4. 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.
  5. 
      
dinos
dinos
Review request changed

Status: Closed (submitted)

Change Summary:

Merged slightly adjusted version into master
Loading...