-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: port DEL command #1445
feat: port DEL command #1445
Conversation
Migrate DEL command to support ironhawk engine
internal/cmd/cmd_del.go
Outdated
return cmdResNil, errWrongArgumentCount("DEL") | ||
} | ||
|
||
var count int64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could be an int32.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arpitbbhayani wire.Response_VInt
supports int64, we can either:
- convert int32 to int64 via int64(count)
- add support for int32 in cmd.proto (wire.Response_VInt32 and Response_VInt64)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need that support. The value of count does not depend on anything in the proto. You will never have massive number of keys being deleted. Change this to int
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
Migrate DEL command to support ironhawk engine.
fixes #1428