Description: |
|
---|
INSERT/REPLACE functionality
Review Request #12 — Created Nov. 7, 2012 and submitted
Information | |
---|---|
Felipe | |
Lunr | |
https://github.com/madsmash/lunr/tree/insert | |
Reviewers | |
lunr | |
Modified Query Builder to support INSERT and REPLACE functionality. Supports using extended inserts, SET and SELECT statements for the data to insert. Set clause can be used in two ways, either with multiple calls like: $builder->set(array('column1'=>'value1'))->set(array('column2'=>'value2'))... AND/OR $builder->set(array('column1'=>'value1', 'column2'=>'value2') Values clause can be used in two ways as well, either with multiple calls like: $builder->values(array('value1col1','value1col2','value1col3'))->values(array('value2col1','value2col2','value2col3')).... AND/OR $builder->values(array(array('value1col1','value1col2','value1col3'),array('value2col1','value2col2','value2col3'))) I will continue making small changes to spam your email until someone reviews it...
All unitests passed
-
-
system/libraries/dataaccess/class.databasedmlquerybuilder.inc.php (Diff revision 1) LOL, awesome typo :P
-
system/libraries/dataaccess/class.databasedmlquerybuilder.inc.php (Diff revision 1) simply append the comma by default and do a trim() at the end
-
-
system/libraries/dataaccess/class.databasedmlquerybuilder.inc.php (Diff revision 1) append by default, trim at the end
-
system/libraries/dataaccess/class.databasedmlquerybuilder.inc.php (Diff revision 1) interesting. I was going to suggest using something else, but it turns out using strpos is the fastest way to check something at the beginning of a string. Basically it's like this: strpos < strncmp < substr I'm pretty sure we use substr in a couple places, so I may have to adjust that ;)
-
system/libraries/dataaccess/class.mysqldmlquerybuilder.inc.php (Diff revision 1) insert into select does not support DELAYED, so you have to implement some kind of mode checking. And since you have to do that anyway, you might just as well merge replace_mode in there as well.
-
I would just say that maybe for end-user it would be more understandable to name the methods and property insert_into or even insert instead of into. otherwise the global seems good. the tests will tell
Change Summary:
Changes requested on review + reformat of the whole file. If there are more formatting issues please let me know so I can adapt the template
Diff: |
Revision 2 (+403 -19) |
---|
-
I still want replace_mode and insert_mode merged. There's no reason to handle them separately.
-
system/libraries/dataaccess/class.databasedmlquerybuilder.inc.php (Diff revisions 1 - 2) putting empty lines between the blocks improves readability
-
system/libraries/dataaccess/class.databasedmlquerybuilder.inc.php (Diff revisions 1 - 2) ok, I'd do a small hack to make the code simpler. If there is only one array level (so $values[0] is not an array), wrap it in an array ($values = array($values);). That way later on you only have to care about multidimensional arrays. I leave it up to you if you like it like that or not :)
-
system/libraries/dataaccess/class.databasedmlquerybuilder.inc.php (Diff revisions 1 - 2) use foreach
-
system/libraries/dataaccess/class.databasedmlquerybuilder.inc.php (Diff revisions 1 - 2) that was actually good before. It improves readability of the code if a group of assignments with reasonably similar length on the left side of the assignment has the '=' aligned.
-
system/libraries/dataaccess/class.databasedmlquerybuilder.inc.php (Diff revisions 1 - 2) why the wrapping?
-
system/libraries/dataaccess/class.databasedmlquerybuilder.inc.php (Diff revisions 1 - 2) keep the empty line pls
Change Summary:
Merged insert_mode and replace_mode, replace_mode() just calls insert_mode() but it's there for consistency.
Diff: |
Revision 4 (+397 -17) |
---|
Change Summary:
Added unitests and fixed small issues I saw while coding them.
Description: |
|
---|
Change Summary:
Added missing unittests for full code coverage
Diff: |
Revision 6 (+1335 -31)
|
---|