Skip to content

Scores support#18

Merged
dengliming merged 4 commits intodengliming:masterfrom
regisoliveira:Scores_Support
Apr 18, 2021
Merged

Scores support#18
dengliming merged 4 commits intodengliming:masterfrom
regisoliveira:Scores_Support

Conversation

@regisoliveira
Copy link
Copy Markdown
Contributor

@regisoliveira regisoliveira commented Apr 18, 2021

SearchResultDecoder crashes when using io.github.dengliming.redismodule.redisearch.search.SearchOptions#withScores equals to true, since the response we get from FT.SEARCH contains the score value just after the document key.

We will calculate the document size using parts data. After them, we use that size to iterate over the parts and build a Document with it's Key a Payload (previous behavior) or Key, Score and Payload (new behavior).

It would be better if we could access the command's parameters inside the SearchResultDecoder to assure what kind of response we are dealing with.

Bellow there are 2 images that shows the parts differences without and with withScores parameter.

Captura de Tela 2021-04-17 às 21 38 44

Captura de Tela 2021-04-17 às 21 37 13

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 18, 2021

Codecov Report

Merging #18 (1561568) into master (2d5eb6c) will decrease coverage by 0.21%.
The diff coverage is 47.36%.

❗ Current head 1561568 differs from pull request most recent head 8330f6b. Consider uploading reports for the commit 8330f6b to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master      #18      +/-   ##
============================================
- Coverage     61.04%   60.83%   -0.22%     
  Complexity      462      462              
============================================
  Files            90       90              
  Lines          1743     1754      +11     
  Branches        152      157       +5     
============================================
+ Hits           1064     1067       +3     
- Misses          587      593       +6     
- Partials         92       94       +2     
Impacted Files Coverage Δ Complexity Δ
...disearch/protocol/decoder/SearchResultDecoder.java 50.00% <47.36%> (-22.73%) 5.00 <4.00> (ø)
.../github/dengliming/redismodule/redisai/Device.java 100.00% <0.00%> (ø) 1.00% <0.00%> (ø%)
...github/dengliming/redismodule/redisai/Backend.java 100.00% <0.00%> (ø) 1.00% <0.00%> (ø%)
...github/dengliming/redismodule/redisai/RedisAI.java 83.56% <0.00%> (ø) 28.00% <0.00%> (ø%)
...ithub/dengliming/redismodule/redisai/DataType.java 100.00% <0.00%> (ø) 1.00% <0.00%> (ø%)
...b/dengliming/redismodule/redisai/model/Tensor.java 100.00% <0.00%> (ø) 4.00% <0.00%> (ø%)
.../dengliming/redismodule/redisbloom/TopKFilter.java 86.36% <0.00%> (ø) 14.00% <0.00%> (ø%)
.../dengliming/redismodule/redisearch/RediSearch.java 53.36% <0.00%> (ø) 47.00% <0.00%> (ø%)
...gliming/redismodule/redisai/args/SetModelArgs.java 47.05% <0.00%> (ø) 9.00% <0.00%> (ø%)
...gliming/redismodule/redisai/protocol/Keywords.java 100.00% <0.00%> (ø) 1.00% <0.00%> (ø%)
... and 73 more

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 2d5eb6c...8330f6b. Read the comment docs.

@dengliming dengliming added RediSearch bug Something isn't working labels Apr 18, 2021
@regisoliveira
Copy link
Copy Markdown
Contributor Author

Done!

@dengliming
Copy link
Copy Markdown
Owner

@regisoliveira Can you resolve conflicts?

Copy link
Copy Markdown
Contributor Author

@regisoliveira regisoliveira left a comment

Choose a reason for hiding this comment

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

Checked!

@regisoliveira
Copy link
Copy Markdown
Contributor Author

@regisoliveira Can you resolve conflicts?

I don't have write access to your repo to solve conflicts.

@dengliming dengliming merged commit d8fea0a into dengliming:master Apr 18, 2021
@dengliming
Copy link
Copy Markdown
Owner

@regisoliveira Thanks. That's merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working RediSearch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants