[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#7570) fix MDB_GET_MULTIPLE and MDB_NEXT_MULTIPLE behaviour



Claude Brisson wrote:
> So it means the following: with the current code, when the client code
> wants to optimize transfers with GET_MULTIPLE/NEXT_MULTIPLE, it has to
> fall back to standard GET/NEXT whenever it encounters MDB_NOTFOUND.
> Whereas the patch assumes that GET_MULTIPLE/NEXT_MULTIPLE will not
> depend on the number of records.
>
> IMHO, the current behavior is a bit cumbersome. Would it be that
> complex to update the client code?

Your patch changes the meaning of these operations. GET_MULTIPLE/NEXT_MULTIPLE 
is *only* for duplicate data and already documented as such. Your patch makes 
them return non-duplicate data. This is a fundamental change in an established 
behavior. A complete break, actually; it will return data that doesn't match 
the key that the client requested. It is completely wrong.

>
>    Claude
>
> On Sun, 14 Apr 2013 23:56:41 GMT
> hyc@symas.com wrote:
>
>> hyc@symas.com wrote:
>>> claude.brisson@gmail.com wrote:
>>>> Full_Name: Claude Brisson
>>>> Version: HEAD
>>>> OS: linux 64
>>>> URL:
>>>> ftp://ftp.openldap.org/incoming/0001-fix-MDB_GET_MULTIPLE-and-MDB_NEXT_MULTIPLE.patch
>>>> Submission from: (NULL) (84.103.74.130)
>>>>
>>>>
>>>> In a db with MDB_DUPSORT flag, when there is only one DUP data for
>>>> a key, F_DUPDATA is not set for this key, and there is no
>>>> sub-cursor.
>>>>
>>>> In this case, the current behaviour of MDB_GET_MULTIPLE and
>>>> MDB_NEXT_MULTIPLE is broken because they make the assumption that
>>>> there is always a sub-cursor for DUP data, which is only true for
>>>> keys with more than one DUP data.
>>>>
>>>> This patch address this specific issue.
>>>
>>> Something's wrong with this patch, it breaks
>>> MDB_GET_MULTIPLE/NEXT_MULTIPLE usage in slapd. Need to investigate
>>> further.
>>>
>> The original code is correct. GET_MULTIPLE/NEXT_MULTIPLE are only
>> valid when there actually are duplicate records. MDB_NEXT_MULTIPLE is
>> essentially just a shortcut for calling MDB_NEXT_DUP in a loop. If
>> there are no dups, then MDB_NEXT_DUP (and MDB_NEXT_MULTIPLE) return
>> MDB_NOTFOUND.
>>
>> This patch was in mdb.master but has now been reverted. Closing this
>> ITS.
>>
>
>


-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/