Skip to content
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

Merged
merged 3 commits into from
Feb 6, 2025
Merged

Conversation

manosriram
Copy link
Contributor

Migrate DEL command to support ironhawk engine.

fixes #1428

Migrate DEL command to support ironhawk engine
return cmdResNil, errWrongArgumentCount("DEL")
}

var count int64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be an int32.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood.

Copy link
Contributor Author

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:

  1. convert int32 to int64 via int64(count)
  2. add support for int32 in cmd.proto (wire.Response_VInt32 and Response_VInt64)

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@arpitbbhayani arpitbbhayani merged commit ccd9d70 into DiceDB:master Feb 6, 2025
0 of 2 checks passed
n4vxn pushed a commit to n4vxn/dice that referenced this pull request Feb 7, 2025
n4vxn pushed a commit to n4vxn/dice that referenced this pull request Feb 7, 2025
KedarisettiSreeVamsi pushed a commit to KedarisettiSreeVamsi/dice that referenced this pull request Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IronHawk Port: DEL command
3 participants