Skip to content

Commit

Permalink
#86: Accept lowercase options for SET
Browse files Browse the repository at this point in the history
Summary: Redis supports lowercase options, and we don't. This fix addresses the issue.

Test Plan:
Before the change:
127.0.0.1:6379> set a 1 ex 1
(error) ERR set: Unidentified argument ex found while parsing set command
127.0.0.1:6379> set a 1 EX 1
OK
127.0.0.1:6379>

After the change:
127.0.0.1:6379> set a 1 ex 1
OK
127.0.0.1:6379> set a 1 EX 1
OK

Reviewers: karthik, pritam.damania, sergei, amitanand

Reviewed By: amitanand

Subscribers: kannan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D4292
  • Loading branch information
hectorgcr committed Mar 8, 2018
1 parent 679b57b commit 897a925
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 5 deletions.
2 changes: 1 addition & 1 deletion java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
<commons-lang3.version>3.6</commons-lang3.version>
<guava.version>16.0.1</guava.version>
<hadoop.version>2.7.3</hadoop.version>
<jedis.version>2.9.0-yb-1</jedis.version>
<jedis.version>2.9.0-yb-2</jedis.version>
<jsr305.version>3.0.1</jsr305.version>
<junit.version>4.12</junit.version>
<log4j.version>1.2.17</log4j.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,36 @@ public void testBasicCommands() throws Exception {
assertEquals("v1", jedis_client.get("k1"));
}

@Test
public void testSetCommandWithOptions() throws Exception {
// Test with lowercase "ex" option.
assertEquals("OK", jedis_client.set("k_ex", "v", "ex", 2));
assertEquals("v", jedis_client.get("k_ex"));
Thread.sleep(2000);
assertEquals(null, jedis_client.get("k_ex"));

// Test with uppercase "EX" option.
assertEquals("OK", jedis_client.set("k_ex", "v", "EX", 2));
assertEquals("v", jedis_client.get("k_ex"));
Thread.sleep(2000);
assertEquals(null, jedis_client.get("k_ex"));

// Test with lowercase "px" option.
assertEquals("OK", jedis_client.set("k_px", "v", "px", 2000));
assertEquals("v", jedis_client.get("k_px"));
Thread.sleep(2000);
assertEquals(null, jedis_client.get("k_px"));

// Test with uppercase "PX" option.
assertEquals("OK", jedis_client.set("k_px", "v", "PX", 2000));
assertEquals("v", jedis_client.get("k_px"));
Thread.sleep(2000);
assertEquals(null, jedis_client.get("k_px"));

// TODO(hector): add tests for options NX and XX once
// https://github.com/YugaByte/yugabyte-db/issues/88 is resolved.
}

@Test
public void testTSAddCommand() throws Exception {
TSValuePairs pairs = new TSValuePairs((1));
Expand Down
14 changes: 10 additions & 4 deletions src/yb/yql/redis/redisserver/redis_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "yb/util/split.h"
#include "yb/util/status.h"
#include "yb/util/stol_utils.h"
#include "yb/util/string_case.h"

namespace yb {
namespace redisserver {
Expand Down Expand Up @@ -110,7 +111,12 @@ CHECKED_STATUS ParseSet(YBRedisWriteOp *op, const RedisClientCommand& args) {
op->mutable_request()->mutable_key_value()->set_type(REDIS_TYPE_STRING);
int idx = 3;
while (idx < args.size()) {
if (args[idx] == "EX" || args[idx] == "PX") {
string upper_arg;
if (args[idx].size() == 2) {
ToUpperCase(args[idx].ToBuffer(), &upper_arg);
}

if (upper_arg == "EX" || upper_arg == "PX") {
if (args.size() < idx + 2) {
return STATUS_SUBSTITUTE(InvalidCommand,
"Expected TTL field after the EX flag, no value found");
Expand All @@ -122,13 +128,13 @@ CHECKED_STATUS ParseSet(YBRedisWriteOp *op, const RedisClientCommand& args) {
"TTL field $0 is not within valid bounds", args[idx + 1]);
}
const int64_t milliseconds_per_unit =
args[idx] == "EX" ? MonoTime::kMillisecondsPerSecond : 1;
upper_arg == "EX" ? MonoTime::kMillisecondsPerSecond : 1;
op->mutable_request()->mutable_set_request()->set_ttl(*ttl_val * milliseconds_per_unit);
idx += 2;
} else if (args[idx] == "XX") {
} else if (upper_arg == "XX") {
op->mutable_request()->mutable_set_request()->set_mode(REDIS_WRITEMODE_UPDATE);
idx += 1;
} else if (args[idx] == "NX") {
} else if (upper_arg == "NX") {
op->mutable_request()->mutable_set_request()->set_mode(REDIS_WRITEMODE_INSERT);
idx += 1;
} else {
Expand Down

0 comments on commit 897a925

Please sign in to comment.