[dev] [commits] Horde branch master updated. f7b75dbcebed550d35027b95d70b65bafb5723bf

[Michael J Rubinsky]

Quoting Thomas Jarosch <thomas.jarosch at intra2net.com>:

> Hi MJR,
>
> On Wednesday, 16. October 2013 14:33:53 you wrote:
>> commit f7b75dbcebed550d35027b95d70b65bafb5723bf
>> Author: Michael J Rubinsky <mrubinsk at horde.org>
>> Date:   Wed Oct 16 10:30:32 2013 -0400
>>
>>     Load all matching map entries for available changes at once.
>>
>>     Don't hit the DB for each change, we already have all the change uids,
>> so fetch all the possible map entries at once.
>>
>>  framework/ActiveSync/lib/Horde/ActiveSync/State/Sql.php |   56
>> ++++++++------- 1 files changed, 31 insertions(+), 25 deletions(-)
>>
>> http://github.com/horde/horde/commit/f7b75dbcebed550d35027b95d70b65bafb572
>> 3bf
>> http://git.horde.org/horde-git/-/commit/f7b75dbcebed550d35027b95d70b65baf
>> b5723bf
>
> hmm, I think you need to tweak the SQL statement for this some more.
> The new code:
>
> +
> +        $conditions = '';
> +        foreach ($changes as $change) {
> +            $d = $change['type'] == Horde_ActiveSync::CHANGE_TYPE_DELETE;
> +            if (strlen($conditions)) {
> +                $conditions .= 'OR ';
> +            }
> +            $conditions .= '(message_uid = ?' . ($d ? ' AND  
> sync_deleted = ?) ' : ') ');
> +            $values[] = $change['id'];
> +            if ($d) {
> +                $values[] = $d;
> +            }
> +        }
> +        $sql .= 'AND ' . $conditions . 'GROUP BY message_uid';
>          try {
> -            return $this->_db->selectValue($sql, $values);
> +            return $this->_db->selectAssoc($sql, $values);
>          } catch (Horde_Db_Exception $e) {
>              throw new Horde_ActiveSync_Exception($e);
>          }
>
>
> -> We need parenthesis around the "AND ($conditions)",
> otherwise we "OR" multiple conditions with the whole SQL statement
> which is not what we want.

Yup. Good catch, thanks. That's what I get for testing against a small  
dataset :)


> If you tweak the code anyway: No need to use strlen()
> to check if a string is empty ;)

Well, we still need to only add the OR after the first set of  
$conditions, so we *do* still need to check that. Either way, I  
refactored that anyway to use an array and explode() instead since  
it's a bit cleaner to read.

Let me know once your testing is (hopefully) successful and I'll  
release the next package. I want to get these fixes out in a released  
package before I merge the 2_9_0 stuff into master.

-- 
mike

The Horde Project (www.horde.org)
mrubinsk at horde.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5849 bytes
Desc: S/MIME Signature
URL: <http://lists.horde.org/archives/dev/attachments/20131017/0faa25f6/attachment.bin>