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

[claude.brisson@gmail.com]

Ok, I understand, now. I totally overlooked the definition of
MDB_NEXT_MULTIPLE, which I believed was here to return all the
duplicate of the next key. Really sorry about all that noise.

You may be interested in keeping the following little change from my
patch, though:

@@ -4377,6 +4377,8 @@ mdb_cursor_next(MDB_cursor *mc, MDB_val *key, MDB_val *data, MDB_cursor_op op)
                        rc = mdb_cursor_first(&mc->mc_xcursor->mx_cursor, data, NULL);
                        if (rc != MDB_SUCCESS)
                                return rc;
+               } else if (mc->mc_db->md_flags & MDB_DUPSORT) {
+                       mc->mc_xcursor->mx_cursor.mc_flags &= ~C_INITIALIZED;
                }
        }

mdb_cursor_next should invalidate mx_cursor when there is no duplicate
data for the current key. It's maybe useless if we are sure that we
won't ever rely on this flag thereafter, but it's certainly safer.

Regards,

  Claude

On Mon, 15 Apr 2013 13:42:40 -0700
Howard Chu <hyc@symas.com> wrote:

> claude.brisson@gmail.com wrote:
> > On Mon, 15 Apr 2013 20:01:01 GMT
> > hyc@symas.com wrote:
> >
> >> Your patch changes the meaning of these operations.
> >> GET_MULTIPLE/NEXT_MULTIPLE is *only* for duplicate data and already
> >> documented as such.
> >
> > The documentation in the header never states that MULTIPLE
> > operations should *fail* on singletons. One could expect that the
> > sentence "Return all the duplicate data items at the current cursor
> > position" means that singletons will *also* be returned, without
> > having to resort to a second call in case of failure.
> >
> >> 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.
> >
> > I'm sorry if it broke something and made you loose your time, I was
> > trying to help here. But the only change is to return singletons
> > instead of MDB_NOTFOUND - the data will always match the key.
> 
> No. This is in your patch:
> 
> @@ -4803,7 +4806,7 @@ mdb_cursor_get(MDB_cursor *mc, MDB_val *key,
> MDB_val *data, if (!(mc->mc_flags & C_INITIALIZED))
>   			rc = mdb_cursor_first(mc, key, data);
>   		else
> -			rc = mdb_cursor_next(mc, key, data,
> MDB_NEXT_DUP);
> +			rc = mdb_cursor_next(mc, key, data,
> MDB_NEXT_NODUP); if (rc == MDB_SUCCESS) {
>   			if (mc->mc_xcursor->mx_cursor.mc_flags &
> C_INITIALIZED) { MDB_cursor *mx;
> 
> So you've made it return non-matching data.
> 
> > Maybe I missed something, here. But I wonder why the client code
> > should at any point *expect* truly duplicate data.
> 
> That is the sole purpose of these options.
> 
> Re-read the description:
> 
>      MDB_GET_MULTIPLE,       /**< Return all the duplicate data items
> at the current
>                                   cursor position. Only for
> #MDB_DUPFIXED */
> 
> If the current cursor position is a singleton, then you already have
> its data if you positioned the cursor using (&key, &data) so there is
> no need to make any further calls anyway.
>