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

Support Vector Similarity #151

Merged
merged 20 commits into from
Apr 24, 2022
Merged

Support Vector Similarity #151

merged 20 commits into from
Apr 24, 2022

Conversation

Avital-Fine
Copy link
Contributor

@Avital-Fine Avital-Fine commented Apr 7, 2022

closes #146

Avital-Fine and others added 3 commits April 7, 2022 13:48

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@Avital-Fine Avital-Fine requested review from filipecosta90, chayim and DvirDukhan and removed request for filipecosta90 April 7, 2022 10:53
@Avital-Fine Avital-Fine marked this pull request as draft April 7, 2022 10:54
Copy link

@DvirDukhan DvirDukhan left a comment

Choose a reason for hiding this comment

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

Looks good
does the param interface{} type support binary blobs?

}

if q.Dialect != 0 {
args = args.Add("DIALECT", q.Dialect)
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to not add dialect anyways?

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 saw that Jedis didn't

chayim
chayim previously approved these changes Apr 7, 2022
@chayim
Copy link
Contributor

chayim commented Apr 7, 2022

@Avital-Fine the code looks right, but did you notice the CI failure? I think it's because we're using redisearch:1.6.15 and you probably want a more modern docker (redis-stack-server or redismod)

@Avital-Fine
Copy link
Contributor Author

@Avital-Fine the code looks right, but did you notice the CI failure? I think it's because we're using redisearch:1.6.15 and you probably want a more modern docker (redis-stack-server or redismod)

How do I update the docker then?
I was trying to add tests with the client now and it is failing

@Avital-Fine Avital-Fine marked this pull request as ready for review April 7, 2022 12:20
@Avital-Fine Avital-Fine requested a review from chayim April 7, 2022 12:20
Avital-Fine added 2 commits April 7, 2022 15:38
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Apr 24, 2022

Codecov Report

Merging #151 (6cb299c) into master (5c6afe8) will increase coverage by 0.41%.
The diff coverage is 91.89%.

@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
+ Coverage   76.44%   76.86%   +0.41%     
==========================================
  Files          13       13              
  Lines        1346     1383      +37     
==========================================
+ Hits         1029     1063      +34     
- Misses        259      261       +2     
- Partials       58       59       +1     
Impacted Files Coverage Δ
redisearch/schema.go 88.29% <85.00%> (-0.40%) ⬇️
redisearch/query.go 87.24% <100.00%> (+1.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c6afe8...6cb299c. Read the comment docs.

@Avital-Fine Avital-Fine merged commit d1b07e7 into RediSearch:master Apr 24, 2022
@gkorland gkorland mentioned this pull request Jul 13, 2022
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.

Add dialect support for RediSearch queries
3 participants