Skip to content

Commit c26e9a3

Browse files
AlexStocksOmX
andauthored
feat(cmd): add TOUCH and SUBSTR commands (#376)
* feat(cmd): add TOUCH and SUBSTR commands TOUCH key [key ...] returns the number of the specified keys that exist, counting each mention separately (Redis semantics). With the cache disabled it is a pure existence count with no last-access side effect. It reuses the existing readonly `Storage::exists`, the same primitive behind EXISTS. SUBSTR key start end is the deprecated alias of GETRANGE, kept for clients written before Redis 2.0. It shares GETRANGE's exact semantics and storage path (`Storage::getrange`). Both are implemented entirely in the cmd layer with no storage-layer changes, completing the multi-key/multi-member readonly family (MGET, HMGET, EXISTS, SMISMEMBER, ZMSCORE, TOUCH). Relates to #144 * fix(string): align substr and getrange with redis Unify SUBSTR and GETRANGE execution, remove the incorrect TOUCH implementation, preserve Redis 8.8.1 negative-index semantics with wide integer arithmetic, and record the deferred manifest status for SUBSTR and TOUCH. Constraint: Limit this follow-up to the readonly string command path, RESP command typing, storage string slicing, and compatibility manifest entries before rebasing onto current main. Confidence: Medium-high; the branch already isolates the reviewed readonly command fixes and the remaining work after commit is mainline sync plus focused verification. Scope-risk: SUBSTR/GETRANGE/TOUCH command behavior and manifest metadata only; no unrelated command families should move. Tested: focused SUBSTR, GETRANGE, RESP mapping, and string-path tests on the pre-rebase patch; git diff --check before branch sync. Not-tested: post-rebase focused verification and manifest/schema checks still need to run after syncing onto current main. Co-authored-by: OmX <omx@oh-my-codex.dev> --------- Co-authored-by: OmX <omx@oh-my-codex.dev>
1 parent 320cd82 commit c26e9a3

8 files changed

Lines changed: 425 additions & 49 deletions

File tree

src/cmd/src/getrange.rs

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -56,48 +56,52 @@ impl Cmd for GetrangeCmd {
5656
}
5757

5858
fn do_cmd(&self, client: &Client, storage: Arc<Storage>) {
59-
let key = client.key();
60-
let argv = client.argv();
59+
execute_getrange(client, storage);
60+
}
61+
}
6162

62-
// Parse start offset
63-
let start = match String::from_utf8_lossy(&argv[2]).parse::<i64>() {
64-
Ok(n) => n,
65-
Err(_) => {
66-
client.set_reply(RespData::Error(
67-
"ERR value is not an integer or out of range".into(),
68-
));
69-
return;
70-
}
71-
};
63+
pub(crate) fn execute_getrange(client: &Client, storage: Arc<Storage>) {
64+
let key = client.key();
65+
let argv = client.argv();
7266

73-
// Parse end offset
74-
let end = match String::from_utf8_lossy(&argv[3]).parse::<i64>() {
75-
Ok(n) => n,
76-
Err(_) => {
77-
client.set_reply(RespData::Error(
78-
"ERR value is not an integer or out of range".into(),
79-
));
80-
return;
81-
}
82-
};
67+
// Parse start offset
68+
let start = match String::from_utf8_lossy(&argv[2]).parse::<i64>() {
69+
Ok(n) => n,
70+
Err(_) => {
71+
client.set_reply(RespData::Error(
72+
"ERR value is not an integer or out of range".into(),
73+
));
74+
return;
75+
}
76+
};
8377

84-
let result = storage.getrange(&key, start, end);
78+
// Parse end offset
79+
let end = match String::from_utf8_lossy(&argv[3]).parse::<i64>() {
80+
Ok(n) => n,
81+
Err(_) => {
82+
client.set_reply(RespData::Error(
83+
"ERR value is not an integer or out of range".into(),
84+
));
85+
return;
86+
}
87+
};
8588

86-
match result {
87-
Ok(substring) => {
88-
client.set_reply(RespData::BulkString(Some(substring.into())));
89-
}
90-
Err(e) => match e {
91-
storage::error::Error::RedisErr { ref message, .. }
92-
if message.starts_with("WRONGTYPE") =>
93-
{
94-
// RedisErr already contains the formatted message
95-
client.set_reply(RespData::Error(message.clone().into()));
96-
}
97-
_ => {
98-
client.set_reply(RespData::Error(format!("ERR {e}").into()));
99-
}
100-
},
89+
let result = storage.getrange(&key, start, end);
90+
91+
match result {
92+
Ok(substring) => {
93+
client.set_reply(RespData::BulkString(Some(substring.into())));
10194
}
95+
Err(e) => match e {
96+
storage::error::Error::RedisErr { ref message, .. }
97+
if message.starts_with("WRONGTYPE") =>
98+
{
99+
// RedisErr already contains the formatted message
100+
client.set_reply(RespData::Error(message.clone().into()));
101+
}
102+
_ => {
103+
client.set_reply(RespData::Error(format!("ERR {e}").into()));
104+
}
105+
},
102106
}
103107
}

src/cmd/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ pub mod srandmember;
8686
pub mod srem;
8787
pub mod sscan;
8888
pub mod strlen;
89+
pub mod substr;
8990
pub mod sunion;
9091
pub mod sunionstore;
9192
pub mod table;

src/cmd/src/substr.rs

Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
// Copyright (c) 2024-present, arana-db Community. All rights reserved.
2+
//
3+
// Licensed to the Apache Software Foundation (ASF) under one or more
4+
// contributor license agreements. See the NOTICE file distributed with
5+
// this work for additional information regarding copyright ownership.
6+
// The ASF licenses this file to You under the Apache License, Version 2.0
7+
// (the "License"); you may not use this file except in compliance with
8+
// the License. You may obtain a copy of the License at
9+
//
10+
// http://www.apache.org/licenses/LICENSE-2.0
11+
//
12+
// Unless required by applicable law or agreed to in writing, software
13+
// distributed under the License is distributed on an "AS IS" BASIS,
14+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
// See the License for the specific language governing permissions and
16+
// limitations under the License.
17+
18+
use std::sync::Arc;
19+
20+
use client::Client;
21+
use storage::storage::Storage;
22+
23+
use crate::getrange::execute_getrange;
24+
use crate::{AclCategory, Cmd, CmdFlags, CmdMeta};
25+
use crate::{impl_cmd_clone_box, impl_cmd_meta};
26+
27+
/// `SUBSTR key start end` is the deprecated alias of `GETRANGE`, kept for
28+
/// compatibility with clients written before Redis 2.0. It shares the exact
29+
/// same semantics and storage path as `GETRANGE`.
30+
#[derive(Clone, Default)]
31+
pub struct SubstrCmd {
32+
meta: CmdMeta,
33+
}
34+
35+
impl SubstrCmd {
36+
pub fn new() -> Self {
37+
Self {
38+
meta: CmdMeta {
39+
name: "substr".to_string(),
40+
arity: 4, // SUBSTR key start end
41+
flags: CmdFlags::READONLY,
42+
acl_category: AclCategory::STRING | AclCategory::READ,
43+
..Default::default()
44+
},
45+
}
46+
}
47+
}
48+
49+
impl Cmd for SubstrCmd {
50+
impl_cmd_meta!();
51+
impl_cmd_clone_box!();
52+
53+
/// SUBSTR key start end
54+
fn do_initial(&self, client: &Client) -> bool {
55+
let argv = client.argv();
56+
let key = argv[1].clone();
57+
client.set_key(&key);
58+
true
59+
}
60+
61+
fn do_cmd(&self, client: &Client, storage: Arc<Storage>) {
62+
execute_getrange(client, storage);
63+
}
64+
}
65+
66+
#[allow(clippy::unwrap_used)]
67+
#[cfg(test)]
68+
mod tests {
69+
use client::StreamTrait;
70+
use resp::{RespData, RespVersion, encode::RespEncode, encode::RespEncoder};
71+
use storage::{StorageOptions, safe_cleanup_test_db, unique_test_db_path};
72+
73+
use super::*;
74+
use crate::table::create_command_table;
75+
76+
struct TestStream;
77+
78+
#[async_trait::async_trait]
79+
impl StreamTrait for TestStream {
80+
async fn read(&mut self, _buf: &mut [u8]) -> Result<usize, std::io::Error> {
81+
Ok(0)
82+
}
83+
84+
async fn write(&mut self, _data: &[u8]) -> Result<usize, std::io::Error> {
85+
Ok(0)
86+
}
87+
}
88+
89+
#[test]
90+
fn test_substr_cmd_meta() {
91+
let cmd = SubstrCmd::new();
92+
assert_eq!(cmd.name(), "substr");
93+
assert_eq!(cmd.meta().arity, 4); // SUBSTR key start end
94+
assert!(cmd.has_flag(CmdFlags::READONLY));
95+
assert!(!cmd.has_flag(CmdFlags::WRITE));
96+
}
97+
98+
#[test]
99+
fn test_substr_cmd_clone() {
100+
let cmd = SubstrCmd::new();
101+
let cloned = cmd.clone_box();
102+
assert_eq!(cloned.name(), cmd.name());
103+
assert_eq!(cloned.meta().arity, cmd.meta().arity);
104+
}
105+
106+
#[test]
107+
fn test_substr_acl_category() {
108+
let cmd = SubstrCmd::new();
109+
assert!(cmd.acl_category().contains(AclCategory::STRING));
110+
assert!(cmd.acl_category().contains(AclCategory::READ));
111+
}
112+
113+
#[test]
114+
fn test_substr_argument_validation() {
115+
let cmd = SubstrCmd::new();
116+
117+
// Valid: command + key + start + end
118+
assert!(cmd.check_arg(4));
119+
120+
// Invalid argument counts
121+
assert!(!cmd.check_arg(3)); // Missing end
122+
assert!(!cmd.check_arg(5)); // Too many arguments
123+
assert!(!cmd.check_arg(1));
124+
assert!(!cmd.check_arg(0));
125+
}
126+
127+
#[tokio::test]
128+
async fn substr_returns_the_redis_raw_bulk_reply_for_extreme_negative_end() {
129+
let db_path = unique_test_db_path();
130+
safe_cleanup_test_db(&db_path);
131+
let mut storage = Storage::new(1, 0);
132+
let _bg_task_rx = storage
133+
.open(Arc::new(StorageOptions::default()), &db_path)
134+
.unwrap();
135+
storage.set(b"key", b"Hello World").unwrap();
136+
let storage = Arc::new(storage);
137+
let client = Client::new(Box::new(TestStream));
138+
client.set_cmd_name(b"substr");
139+
client.set_argv(&[
140+
b"substr".to_vec(),
141+
b"key".to_vec(),
142+
b"0".to_vec(),
143+
b"-100".to_vec(),
144+
]);
145+
146+
let command_table = create_command_table(Arc::new(|| None));
147+
command_table
148+
.get("substr")
149+
.expect("SUBSTR should be publicly registered")
150+
.execute(&client, Arc::clone(&storage));
151+
152+
let reply = client.take_reply();
153+
assert_eq!(reply, RespData::BulkString(Some(b"H".to_vec().into())));
154+
for version in [RespVersion::RESP2, RespVersion::RESP3] {
155+
let mut encoder = RespEncoder::new(version);
156+
encoder.encode_resp_data(&reply);
157+
assert_eq!(encoder.get_response().as_ref(), b"$1\r\nH\r\n");
158+
}
159+
160+
drop(storage);
161+
safe_cleanup_test_db(&db_path);
162+
}
163+
164+
#[tokio::test]
165+
async fn substr_preserves_wrongtype_error_prefix() {
166+
let db_path = unique_test_db_path();
167+
safe_cleanup_test_db(&db_path);
168+
let mut storage = Storage::new(1, 0);
169+
let _bg_task_rx = storage
170+
.open(Arc::new(StorageOptions::default()), &db_path)
171+
.unwrap();
172+
storage.hset(b"hash", b"field", b"value").unwrap();
173+
let storage = Arc::new(storage);
174+
let client = Client::new(Box::new(TestStream));
175+
client.set_cmd_name(b"substr");
176+
client.set_argv(&[
177+
b"substr".to_vec(),
178+
b"hash".to_vec(),
179+
b"0".to_vec(),
180+
b"-1".to_vec(),
181+
]);
182+
183+
SubstrCmd::new().execute(&client, Arc::clone(&storage));
184+
185+
let reply = client.take_reply();
186+
let RespData::Error(message) = &reply else {
187+
panic!("expected WRONGTYPE error, got {reply:?}");
188+
};
189+
assert!(message.starts_with(b"WRONGTYPE"));
190+
let mut encoder = RespEncoder::new(RespVersion::RESP2);
191+
encoder.encode_resp_data(&reply);
192+
assert!(encoder.get_response().starts_with(b"-WRONGTYPE"));
193+
194+
drop(storage);
195+
safe_cleanup_test_db(&db_path);
196+
}
197+
198+
#[tokio::test]
199+
async fn substr_preserves_binary_values_and_returns_empty_for_missing_keys() {
200+
let db_path = unique_test_db_path();
201+
safe_cleanup_test_db(&db_path);
202+
let mut storage = Storage::new(1, 0);
203+
let _bg_task_rx = storage
204+
.open(Arc::new(StorageOptions::default()), &db_path)
205+
.unwrap();
206+
storage.set(b"binary", b"\xff\x00").unwrap();
207+
let storage = Arc::new(storage);
208+
let client = Client::new(Box::new(TestStream));
209+
210+
for (key, expected) in [
211+
(
212+
b"binary".as_slice(),
213+
RespData::BulkString(Some(vec![0xff].into())),
214+
),
215+
(
216+
b"missing".as_slice(),
217+
RespData::BulkString(Some(Vec::new().into())),
218+
),
219+
] {
220+
client.set_cmd_name(b"substr");
221+
client.set_argv(&[
222+
b"substr".to_vec(),
223+
key.to_vec(),
224+
b"0".to_vec(),
225+
b"0".to_vec(),
226+
]);
227+
SubstrCmd::new().execute(&client, Arc::clone(&storage));
228+
assert_eq!(client.take_reply(), expected);
229+
}
230+
231+
drop(storage);
232+
safe_cleanup_test_db(&db_path);
233+
}
234+
235+
#[test]
236+
fn substr_rejects_out_of_range_integer_arguments_before_storage_access() {
237+
let client = Client::new(Box::new(TestStream));
238+
client.set_cmd_name(b"substr");
239+
client.set_argv(&[
240+
b"substr".to_vec(),
241+
b"key".to_vec(),
242+
b"9223372036854775808".to_vec(),
243+
b"0".to_vec(),
244+
]);
245+
246+
SubstrCmd::new().execute(&client, Arc::new(Storage::new(1, 0)));
247+
248+
assert_eq!(
249+
client.take_reply(),
250+
RespData::Error("ERR value is not an integer or out of range".into())
251+
);
252+
}
253+
}

src/cmd/src/table.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ pub fn create_command_table(requirepass_provider: RequirepassProvider) -> CmdTab
6565
crate::decr::DecrCmd,
6666
crate::decrby::DecrbyCmd,
6767
crate::strlen::StrlenCmd,
68+
crate::substr::SubstrCmd,
6869
crate::getrange::GetrangeCmd,
6970
crate::setrange::SetrangeCmd,
7071
crate::setex::SetexCmd,
@@ -202,6 +203,14 @@ mod tests {
202203

203204
use super::create_command_table;
204205

206+
#[test]
207+
fn registers_substr_but_not_touch_until_access_metadata_exists() {
208+
let table = create_command_table(Arc::new(|| None));
209+
210+
assert!(table.contains_key("substr"));
211+
assert!(!table.contains_key("touch"));
212+
}
213+
205214
struct TestStream;
206215

207216
#[async_trait::async_trait]

0 commit comments

Comments
 (0)