I think i don't like The end can be empty, that means the default end will take max length of key..
Since we already have the getrange test <start> -1, adding it would be more expensive to understand.

I think i don't like The end can be empty, that means the default end will take max length of key.. Since we already have the getrange test <start> -1, adding it would be more expensive to understand.

It use to solve a problem about Change the end inclusive to exclusive.
For example, when the end exclusive, getrange test <start> -1 will not take the last one.

127.0.0.1:6379> get test
"0123456789"
127.0.0.1:6379> getrange test -3 -1
"78"

If we use sentence like follow, I think it may ambiguous, so use empty end to instead of ''getrange test <start> -1.

127.0.0.1:6379> getrange test -3 0
"789"

Maybe there is another way to solve it, please give any ideas.

IMHO, I'm not a fan of 1 and 3.
Isn't our goal to return empty string when start > end?

As mentioned in this comment: #11738 (comment)
We just let end to be non-inclusive.
What about the following patch:

diff --git a/src/t_string.c b/src/t_string.c
index 2bce3acc..2ef6e339 100644
--- a/src/t_string.c
+++ b/src/t_string.c
@@ -527,8 +527,11 @@ void getrangeCommand(client *c) {
     }
     if (start < 0) start = strlen+start;
     if (end < 0) end = strlen+end;
+    if (end < 0) {
+        addReply(c,shared.emptybulk);
+        return;
+    }
     if (start < 0) start = 0;
-    if (end < 0) end = 0;
     if ((unsigned long long)end >= strlen) end = strlen-1;

Yes, your idea is more simple. Let me make a quick change.

Outright changing the meaning of a parameter (1) is a complete no-go. That's a hard break. If there is a valid reason, a valid suggestion might be to allow a syntax to optionally change it, i.e. like the ( in zrange, but honestly: I don't see the point of that in the case of indices (as opposed to scores): just request one fewer

@linzihao1999 I think you misunderstood non-inclusive, what I meant was that it shouldn't be set to 0 when end < 0, and like start, it can't be strlen when start >= strlen.

Another thought:
When entering the last if, the interval of start is [0, ∞] and the interval of end is [-∞, strlen-1].
So all we have to do is add in whether one of them is out of range, and we're done.

diff --git a/src/t_string.c b/src/t_string.c
index 2bce3acc8..8b887a8e9 100644
--- a/src/t_string.c
+++ b/src/t_string.c
@@ -520,20 +520,14 @@ void getrangeCommand(client *c) {
         strlen = sdslen(str);
     }
 
-    /* Convert negative indexes */
-    if (start < 0 && end < 0 && start > end) {
-        addReply(c,shared.emptybulk);
-        return;
-    }
     if (start < 0) start = strlen+start;
     if (end < 0) end = strlen+end;
     if (start < 0) start = 0;
-    if (end < 0) end = 0;
     if ((unsigned long long)end >= strlen) end = strlen-1;
 
     /* Precondition: end >= 0 && end < strlen, so the only condition where
      * nothing can be returned is: start > end. */
-    if (start > end || strlen == 0) {
+    if ((unsigned long long)start >= strlen || end < 0 || start > end || strlen == 0) {
         addReply(c,shared.emptybulk);
     } else {
         addReplyBulkCBuffer(c,(char*)str+start,end-start+1);

@sundb Thanks for patient guidance!
I make a new change, this should fix the bug and not a hard break.

@linzihao1999 please also update the top comment with a brief description of break changes.
@oranagra It's ok for merge.

Hi @sundb top comment updated

@linzihao1999 thanks.
AFAICT i think the current version looks good.

i think it was me that suggested that all the headache with this command comes from the fact end is inclusive, and IIRC my suggestion was to first convert the inputs in the beginning of the function so that end is non-inclusive, and then apply a set of corrections (for negative numbers and so on), so that the rest of the code will behave more logically.

in any case, we can't completely change the meaning of the command arguments, if we wanna do that we need to deprecate the old command and invent a new one.

looking at the recent revision in the code, i understand that you didn't change the meaning of the arguments, and the behavior change here is minimal. i.e. it'll only change behavior when the user used a very specific unlikely scenario when both start and end are <=0 and end >= start.
is that right? if it is (or isn't) please edit the top comment to explicitly state the conditions in which the result is different than before. (to supplement the example you gave).

i also see the top comment still mention something about changing start and end to non-inclusive. is that a relic from a previous revision? it's confusing...
maybe you should split the top comment into two parts, one meant for users and describing the interface changes, and another about the internal mechanics of the code you wrote (i.e. be very clear that this non-inclusive (if at all still relevant) is about the variables in the function, and not the command arguments))

we discussed this in a core-team meeting, we do agree it is worth fixing. (as a breaking change to be released in 8.0).

@linzihao1999 please ack on my previous message and see if you can make the top comment clearer.

@oranagra the comment has been updated.

is the behavior change only when end < -strlen? or like i wrote above when start <=0 && end <=0 && end >= start? (don't remember how i got to that conclusion).
just trying to make the behavior change statement clear and accurate...

is the behavior change only when end < -strlen?

all the changes were made for it and judge if it was out of range before set end to 0.
the other changes are just changing the position.

All committers have signed the CLA.