-
Notifications
You must be signed in to change notification settings - Fork 62
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
Support DROPINDEX command with DD option #170
base: master
Are you sure you want to change the base?
Conversation
@@ -324,6 +324,8 @@ | |||
*/ | |||
boolean dropIndex(boolean missingOk); | |||
|
|||
boolean dropIndexDD(); |
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.
@sazzad16 should you also add "boolean missingOk"?
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.
@gkorland IMHO, if this is a necessity, it should be supported from RediSearch. If too much convenience is given to users, there is a way greater chance of it being misused.
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.
why to introduce new method?
why not to add overloaded method
boolean dropIndex(boolean missingOk, boolean keepDocuments)
dropIndexDD
name is confusing
and client still use deprecated FT.DROP -> which use different parameter
assertUnknownIndex(jde); | ||
} | ||
try { | ||
search.getDocuments("doc2", "doc3"); |
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.
Please add another test that also verify docs were deleted with EXISTS or HGET
Resolves #169