[GitHub] [incubator-kvrocks] PragmaTwice opened a new issue, #598: A unified abstraction for redis command parsing

[GitBox <gi...@apache.org>]

PragmaTwice opened a new issue, #598:
URL: https://github.com/apache/incubator-kvrocks/issues/598

   ### Search before asking
   
   - [X] I had searched in the [issues](https://github.com/apache/incubator-kvrocks/issues) and found no similar issues.
   
   
   ### Motivation
   
   **comments from @PragmaTwice:**
   I feel that it might be a bit too cumbersome to operate a `vector<string>` for each command to parse, and there would be a lot of duplicated logic. It might be nice to design some unified abstraction, but I haven't figured out how to do that yet.
   
   Here are some initial ideas.
   
   To parse `SET key value [ NX | XX] [GET] [ EX seconds | PX milliseconds | EXAT unix-time-seconds | PXAT unix-time-milliseconds | KEEPTTL]` ([ref](https://redis.io/commands/set/)):
   
   ```
   #define CHECK(x) if(!x.ok()) return parse failed
   #define CHECK_ASSIGN(x, v, default) if(x == default || x == v) x = v else return parse failed
   
   iter = parsing iterator(args...);
   
   CHECK(iter.pick_str(&key)); // insure the arg exists, eat it and dump to `key` if exists
   CHECK(iter.pick_str(&value));
   
   while(!iter.reach_end()) {
     auto pos = iter.get_pos();
   
     if(iter.pick_eq("NX")) { // check the current arg is NX, eat it and return true if it is, otherwise return false
       CHECK_ASSIGN(set_flag, nx, none)
     } else if(iter.pick_eq("XX")) {
       CHECK_ASSIGN(set_flag, xx, none)
     }
   
     if(iter.pick_eq("GET")) {
        get_flag = true;
     }
     
     if(iter.pick_eq("EX")) {
       CHECK_ASSIGN(expire_flag, ex, none);
       CHECK(iter.pick_int(&expire_value));
     } else if(iter.pick_eq("PX")) {
       CHECK_ASSIGN(expire_flag, px, none);
       CHECK(iter.pick_int(&expire_value));
     } else if(iter.pick_eq("EXAT")) {
       CHECK_ASSIGN(expire_flag, exat, none);
       CHECK(iter.pick_int(&expire_value));
     } else if(iter.pick_eq("PXAT")) {
       CHECK_ASSIGN(expire_flag, pxat, none);
       CHECK(iter.pick_int(&expire_value));
     } else if(iter.pick_eq("KEEPTTL")) {
       CHECK_ASSIGN(expire_flag, keepttl, none);
     }
   
     if(iter.at(pos)) return parse failed;
   }
   ```
   
   or a shorter version with string literal as enums:
   
   ```
   #define CHECK_ASSIGN(x, v) if(x == nullptr || x == v) x = v else return parse failed
   
   const char *iter.pick_eq_literals(const char *strs[]) {
     for(str : strs) {
       if(iter.pick_eq(str)) return str;
     }
     return nullptr;
   }
   
   iter = parsing iterator(args...);
   
   CHECK(iter.pick_str(&key));
   CHECK(iter.pick_str(&value));
   
   while(!iter.reach_end()) {
     auto pos = iter.get_pos();
   
     if(new_set_flag = iter.pick_eq_literals("NX", "XX")) {
       CHECK_ASSIGN(set_flag, new_set_flag)
     }
   
     if(iter.pick_eq("GET")) {
        get_flag = true
     }
   
     if(new_expire_flag = iter.pick_eq_literals("EX", "PX", "EXAT", "PXAT", "KEEPTTL")) {
       CHECK_ASSIGN(expire_flag, new_expire_flag);
     }
     
     if(new_expire_flag && new_expire_flag != "KEEPTTL"s) {
       CHECK(iter.pick_int(&expire_value));
     }
   
     if(iter.at(pos)) return parse failed;
   }
   ```
   
   We can design it as an utility class, so command authors are free to use it or not.
   
   Advantages:
   - do not need to maintain the parsing state by ourselves (mostly)
   - do not need to parse integer, float, or any input type by ourselves; report parse error for these types
   - as efficient as previous
   
   What do you think? I would appreciate any comments! @git-hulk  @ShooterIT 
   
   **comments from @git-hulk**
   It looks better than before, this idea is really nice to me.
   
   _Originally posted by @ShooterIT in https://github.com/apache/incubator-kvrocks/issues/456#issuecomment-1132391216_
   
   ### Solution
   
   _No response_
   
   ### Are you willing to submit a PR?
   
   - [X] I'm willing to submit a PR!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@kvrocks.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kvrocks] PragmaTwice closed issue #598: A unified abstraction for redis command parsing

[GitBox <gi...@apache.org>]

PragmaTwice closed issue #598: A unified abstraction for redis command parsing
URL: https://github.com/apache/incubator-kvrocks/issues/598


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org