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

Add vector index capabilities #3

Closed
wants to merge 45 commits into from
Closed

Add vector index capabilities #3

wants to merge 45 commits into from

Conversation

swilly22
Copy link
Contributor

No description provided.

Copy link
Contributor

@AviAvni AviAvni left a comment

Choose a reason for hiding this comment

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

partial review see comments

src/arithmetic/vector_funcs/vector_funcs.c Outdated Show resolved Hide resolved
src/arithmetic/vector_funcs/vector_funcs.c Outdated Show resolved Hide resolved
src/arithmetic/vector_funcs/vector_funcs.c Outdated Show resolved Hide resolved
src/procedures/proc_vector_create_index.c Outdated Show resolved Hide resolved
src/graph/graphcontext.c Outdated Show resolved Hide resolved
src/graph/graphcontext.c Outdated Show resolved Hide resolved
src/graph/graphcontext.c Outdated Show resolved Hide resolved
tests/flow/test_effects.py Show resolved Hide resolved
//--------------------------------------------------------------------------

Index idx = NULL;
if(t == INDEX_FLD_RANGE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

move to switch case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe I can as INDEX_FLD_RANGE isn't an enum value but a macro oring 3 different enum values.
switch case can't have INDEX_FLD_RANGE as one of its cases

IDX_FULLTEXT = 2,
IDX_ANY = 0,
IDX_EXACT_MATCH = 1,
IDX_FULLTEXT = 2,
Copy link
Contributor

Choose a reason for hiding this comment

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

is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe it is, old encoded RDBs will contain index type.

t == INDEX_FLD_RANGE ||
t == INDEX_FLD_VECTOR);

if(t == INDEX_FLD_FULLTEXT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

move to switch case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe I can as INDEX_FLD_RANGE isn't an enum value but a macro oring 3 different enum values.
switch case can't have INDEX_FLD_RANGE as one of its cases

a->type |= b->type;

// merge options
if(b->type & INDEX_FLD_FULLTEXT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

move to switch case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe I can as INDEX_FLD_RANGE isn't an enum value but a macro oring 3 different enum values.
switch case can't have INDEX_FLD_RANGE as one of its cases

ErrorCtx_SetError(EMSG_MUST_BE, "Stopword", "string");
return PROCEDURE_ERR;
}
if(!SIArray_AllOfType(sw, T_STRING)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this function now same as in index_fulltext_create.c?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar, this one knows how to handle all of the different configuration one can specify when creating a full-text index with multiple fields, where each field has its own configuration, this is not possible with the CREATE FULLTEXT INDEX syntax

RedisModuleIO *rdb
);

void RdbLoadNodes_v15
Copy link
Contributor

Choose a reason for hiding this comment

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

why we need version 15 you added version 14 in this pr

@AviAvni AviAvni closed this Oct 29, 2023
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.

None yet

2 participants