ShooterIT commented on issue #456: URL: https://github.com/apache/incubator-kvrocks/issues/456#issuecomment-1132391216
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 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
