search

danitseitlin / redis-modules-sdk-ts

A Software development kit for easier connection and execution of Redis Modules commands.
Apache License 2.0
185 stars 29 forks source link

RediSearch incorrectly names `Separator` as `Seperator` #181

Closed RShohoney closed 2 years ago

RShohoney commented 2 years ago

The title explains this pretty plainly. Seperator is misspelled, and breaks the usages of SEPARATOR for tags. It seems that maybe the new version of ReJSON and RediSearch stopped defaulting the SEPARATOR value, as well. When running FT.INFO against a newly created index with tags, the SEPARATOR value for each tag is empty, and tags do not behave as expected (with comma being default). When forcing the correct SEPARATOR property, the TAGS work as expected, and things like FT.TAGVALS return the expected SET of values. FT.INFO also correctly lists the SEPARATOR value for each tag. Additionally, when passing SEPERATOR as an option on an Tag index field, an error will be thrown from redis for using an unknown property.

danitseitlin commented 2 years ago

@RShohoney Hi, thanks for the issue. Will look into it ASAP. You are also welcome to contribute a fix yourself if that's something you'd like.

RShohoney commented 2 years ago

Sure! I'd be happy to take a look. I do have a question about it. I'm assuming that we might not want to break the existing API where the variables names are misspelled? We can just patch the attribute that's passed in the FT.CREATE command, or we can patch all of the variable names, as well. What does the team think?

danitseitlin commented 2 years ago

@RShohoney From an overall view it seems like we just need to adjust the name of the keyword and the type value to the correct name

danitseitlin commented 2 years ago

@RShohoney Hi! are you planning to work on it? or should I?

RShohoney commented 2 years ago

Hi @danitseitlin , taking a look today. Sorry for the delay, I've been unavailable. I expect to have something up today.

RShohoney commented 2 years ago

@danitseitlin, I am unable to push my fix branch to the repository. I didn't see any specifics on PR creation, outside of general rules. Is there something I'm missing?

danitseitlin commented 2 years ago

@RShohoney You need to do it via Fork