-
Notifications
You must be signed in to change notification settings - Fork 140
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
Labels support #22
Labels support #22
Conversation
…ebylabels`, simpler index query logic
…fix strtok_r usage and rename vars
src/indexer.c
Outdated
RedisModule_StringPtrLen(labels[i].value, &_s)); | ||
RedisModuleString *indexed_key = RedisModule_CreateStringPrintf(ctx, "__index_%s", | ||
RedisModule_StringPtrLen(labels[i].key, &_s), | ||
RedisModule_StringPtrLen(labels[i].value, &_s)); |
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.
this line should be removed, only key is expected
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.
also, let's not call RedisModule_StringPtrLen(labels[i].key, &_s) twice
src/indexer.c
Outdated
if (*lastKey == NULL) { | ||
iter = RedisModule_DictIteratorStartC(left, "^", NULL, 0); | ||
} else { | ||
iter = RedisModule_DictIteratorStartC(left, "^", lastKey, *lastKeySize); |
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.
I'm not sure what this line is trying to do, but RedisModule_DictIteratorStartC with "^" ignores the given key
} else if (predicate->type == EQ) { | ||
return localResult; | ||
} else { | ||
return prevResults; |
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.
if you try to find all keys that don't match a label , and prevResults is NULL, this will return NULL instead of all the keys
@@ -0,0 +1,184 @@ | |||
#include <string.h> |
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 documentation to indexer.c
compactedLabels[i].key = RedisModule_CreateStringFromString(NULL, labels[i].key); | ||
compactedLabels[i].value = RedisModule_CreateStringFromString(NULL, labels[i].value); | ||
} | ||
|
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.
a comment explaining what we do here could be nice
… into labels # Conflicts: # README.md
* CircleCI: build multiarch docker image * CircleCI: build multiarch docker image #2 * CircleCI: build multiarch docker image #3 * CircleCI: build multiarch docker image #4 * CircleCI: build multiarch docker image #5 * CircleCI: build multiarch docker image #6 * CircleCI: build multiarch docker image #7 * CircleCI: build multiarch docker image #8 * CircleCI: build multiarch docker image #9 * CircleCI: build multiarch docker image #10 * CircleCI: build multiarch docker image #11 * CircleCI: build multiarch docker image #12 * CircleCI: build multiarch docker image #13 * CircleCI: build multiarch docker image #14 * CircleCI: build multiarch docker image #15 * CircleCI: build multiarch docker image #16 * CircleCI: build multiarch docker image #17 * CircleCI: build multiarch docker image #18 * CircleCI: build multiarch docker image #19 * CircleCI: build multiarch docker image #20 * CircleCI: build multiarch docker image #21 * CircleCI: build multiarch docker image #22 * CircleCI: build multiarch docker image #23 * CircleCI: build multiarch docker image #24 * CircleCI: build multiarch docker image #25 * CircleCI: build multiarch docker image #26 * CircleCI: build multiarch docker image #27 * CircleCI: build multiarch docker image #28 * CircleCI: build multiarch docker image * CircleCI: build multiarch docker image #2 * CircleCI: build multiarch docker image #3 * CircleCI: build multiarch docker image #4 * CircleCI: build multiarch docker image #5 * CircleCI: build multiarch docker image #6 * CircleCI: build multiarch docker image #7 * CircleCI: build multiarch docker image #8 * CircleCI: build multiarch docker image #9 * CircleCI: build multiarch docker image #10 * CircleCI: build multiarch docker image #11 * CircleCI: build multiarch docker image #12 * CircleCI: build multiarch docker image #13 * CircleCI: build multiarch docker image #14 * CircleCI: build multiarch docker image #15 * CircleCI: build multiarch docker image #16 * CircleCI: build multiarch docker image #17 * CircleCI: build multiarch docker image #18 * CircleCI: build multiarch docker image #19 * CircleCI: build multiarch docker image #20 * CircleCI: build multiarch docker image #21 * CircleCI: build multiarch docker image #22 * CircleCI: build multiarch docker image #23 * CircleCI: build multiarch docker image #24 * CircleCI: build multiarch docker image #25 * CircleCI: build multiarch docker image #26 * CircleCI: build multiarch docker image #27 * CircleCI: build multiarch docker image #28 * Readies sync
No description provided.