Any reason why LINSERT is not variadic?

57 views
Skip to first unread message

Pierre Chapuis

unread,
Jul 20, 2013, 8:08:28 AM7/20/13
to redi...@googlegroups.com
Looking at the documentation of LINSERT, I can see no reason why it could not be made variadic without breaking compatibility, i.e.:

    LINSERT key BEFORE|AFTER pivot value [value...]

Am I overlooking something?

The only difficulty here is the behavior of LINSERT key BEFORE pivot v1 v2. Does it result it:

    1) [... v1 v2 pivot...], or
    2) [... v2 v1 pivot...]

I would personally prefer solution 2 which is coherent with the behavior of LPUSH.

If nobody sees an issue with it, I propose to implement it and submit a pull request.
But I know Antirez likes to discuss things here first, hence this message :)

So, thoughts?

-- 
Pierre Chapuis

Salvatore Sanfilippo

unread,
Aug 5, 2013, 8:37:54 AM8/5/13
to Redis DB
Hello Pierre,

I believe there is nothing wrong with a variadic LINSERT, as you noted
backward compatibility is ok and the return value also makes sense in
the new form.

However about the behavior when BEFORE is used, I disagree with your
proposal as IMHO LINSERT is different than a variadic LPOP. "Before"
does not make it equivalent to pushing elements to the left/head, but
I read it as:

Please List-INSERT the following sublist A B C D BEFORE element 5

So if my list is 3 4 5 6 7 8 I expect the result to be:

3 4 A B C D 5 6 7 8

In general I expect LINSERT A B C D to insert the sequence in the same
order regardless of after/before, so the operation should be
semantically coherent, regardless of *where* the insertion point is.

That said, for all the rest, it looks like an ok change to me.

Thanks,
Salvatore
> --
> You received this message because you are subscribed to the Google Groups
> "Redis DB" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to redis-db+u...@googlegroups.com.
> To post to this group, send email to redi...@googlegroups.com.
> Visit this group at http://groups.google.com/group/redis-db.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>



--
Salvatore 'antirez' Sanfilippo
open source developer - GoPivotal
http://invece.org

Beauty is more important in computing than anywhere else in technology
because software is so complicated. Beauty is the ultimate defence
against complexity.
— David Gelernter

Pierre Chapuis

unread,
Aug 5, 2013, 8:43:24 AM8/5/13
to redi...@googlegroups.com
Le lundi 5 août 2013 14:37:54 UTC+2, Salvatore Sanfilippo a écrit :
Hello Pierre,

I believe there is nothing wrong with a variadic LINSERT, as you noted
backward compatibility is ok and the return value also makes sense in
the new form.

However about the behavior when BEFORE is used, I disagree with your
proposal as IMHO LINSERT is different than a variadic LPOP. "Before"
does not make it equivalent to pushing elements to the left/head, but
I read it as:

Please List-INSERT the following sublist A B C D BEFORE element 5

So if my list is 3 4 5 6 7 8 I expect the result to be:

3 4 A B C D 5 6 7 8

In general I expect LINSERT A B C D to insert the sequence in the same
order regardless of after/before, so the operation should be
semantically coherent, regardless of *where* the insertion point is.

That said, for all the rest, it looks like an ok change to me.

OK. I will try implementing this as soon as I find a little time to do it.

Just a precision about the advantage of doing this compared to a pipeline
or a Lua script: not only does it have a simpler interface, but I think it will
also make the complexity of inserting K successive elements in a list of
N elements O(N+K) instead of O(N*K).
 

Pierre Chapuis

unread,
Aug 5, 2013, 8:53:53 PM8/5/13
to redi...@googlegroups.com
Le lundi 5 août 2013 14:43:24 UTC+2, Pierre Chapuis a écrit :

OK. I will try implementing this as soon as I find a little time to do it.

Just a precision about the advantage of doing this compared to a pipeline
or a Lua script: not only does it have a simpler interface, but I think it will
also make the complexity of inserting K successive elements in a list of
N elements O(N+K) instead of O(N*K).
 

Just submitted a pull request: https://github.com/antirez/redis/pull/1232

The first three commits can probably be merged. They separate the implementation
of LINSERT from that of LPUSHX and RPUSHX and make those commands variadic
as a bonus.

The fourth commit (actual implementation of variadic LINSERT) is more convoluted
because of a subtlety with iterators on ziplist (or maybe something I did not understand
about them...). So that one clearly needs to be reviewed and probably re-worked before
being merged. It is completely independent from the 3 other ones which could be
cherry-picked if needed.

Reply all
Reply to author
Forward
0 new messages