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 for redisearch? #10

Open
lumina7 opened this issue Mar 19, 2020 · 5 comments
Open

Support for redisearch? #10

lumina7 opened this issue Mar 19, 2020 · 5 comments

Comments

@lumina7
Copy link

lumina7 commented Mar 19, 2020

Hello, great module :)

Redisearch command works but some of the results are marked as 'undef'.

https://oss.redislabs.com/redisearch/Quick_Start.html

git clone --recursive https://github.com/RediSearch/RediSearch.git
cd RediSearch
make build
make run
use Redis::hiredis;
use Data::Dumper;

my $redis = Redis::hiredis->new();
   $redis->connect('127.0.0.1', 6379);

# create index
$redis->command("FT.CREATE my_index SCHEMA title TEXT body TEXT");

# add documents
$redis->command("FT.ADD my_index doc1 1.0 FIELDS title foo body bar");
$redis->command("FT.ADD my_index doc2 1.0 FIELDS title foo_2 body bar_2");

# search
my $search = $redis->command("FT.SEARCH my_index foo");

print Dumper(\$search);

$VAR1 = \[
            1,
            'doc1',
            undef
          ];

On redis-cli I get.

1) (integer) 1
2) "doc1"
3) 1) "title"
   2) "foo"
   3) "body"
   4) "bar"

Looks like some array are not getting parsed. Redisearch is pretty great and we have 0 perl client, a fix would be appreciated.

@neophenix
Copy link
Owner

hiredis isn't even listed as a client on the redisearch site so not sure if this is something I can do or not. This lib is just a wrapper on that, so if it doesn't support it this lib wouldn't either. Not sure when I would be able to try it, but the most I can really hope for here is that an upgrade of the hiredis lib leads to it "just working"

@lumina7
Copy link
Author

lumina7 commented Mar 19, 2020

Thank you.
As this works on redis-cli, the last version of hiredis should work "finger crossed"

This really looks like a very trivial fix but I am totally clueless with C.

@lumina7
Copy link
Author

lumina7 commented Mar 19, 2020

Redisearch do work with hiredis (v0.14.0) and an older one (v0.11.0). 0.11 is very old, so I don't think upgrading will have any change. I could not figure which one was bundled with the package.

a gist of working hiredis with redisearch:
https://gist.github.com/michael-grunder/0a6a49b39660c9d55e13c551ca78b2ee

The problem probably comes because the redisearch reponse is a mix of array of array and array elements, and that usually never happen in classic redis.

Returns¶
Array reply, where the first element is the total number of results, and then pairs of document id, and a nested array of field/value.

@lumina7
Copy link
Author

lumina7 commented Mar 19, 2020

The module (at least the one in cpan) is using hiredis 0.11.0, which is working with redisearch, the issue is with the wrapper...

https://metacpan.org/source/NEOPHENIX/Redis-hiredis-0.11.0/hiredis.h#L38

@neophenix
Copy link
Owner

neophenix commented Apr 11, 2020

Well for whatever reason I didn't see your replies about why it wasn't working and went ahead with the hiredis upgrade tonight.

Honestly, I am not sure I really have the interest in digging much further into it since I no longer use Perl or Redis. I'd gladly merge a PR if the tests are passing, but beyond that I really don't see me doing much with it, sorry.

Edit: however, the file you linked to is not my code, that is taken from hiredis, so maybe this will work, I'm interested to know since I did not test this specific use, just made sure make test still worked

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

No branches or pull requests

2 participants