tidwall / redcon

Redis compatible server framework for Go
MIT License
2.16k stars 159 forks source link

Discovered a bug #39

Closed gitmko0 closed 3 years ago

gitmko0 commented 3 years ago
<?php

        $qrediswp = new Redis();
        $qrediswp->connect("127.0.0.1", 6379); //dashboard host, port

//hset "e" "432232157184245456" "123456789"

        $lss_disksize = $qrediswp->hget("e","432232157184245456");
        echo '= '.$lss_disksize.' =';

// it shows :123456789 instead of 123456789.
why is there a ":" in front of the value? it doesnt show this using the real redis server.

        exit;

Tried with this code...

//shows up using this:

                case "hget":

                        if bytes.Compare(cmd.Args[1],[]byte("e"))==0 { //disk space
                                        conn.WriteUint64(uint64(123456789))

                       }
tidwall commented 3 years ago

why is there a ":" in front of the value?

The ":" indicates that the value was a Redis Integer. You used conn.WriteUint64, which will write a Redis Integer.

it doesnt show this using the real redis server.

A real Redis server does not use integers for HSET and HGET commands. It uses strings.

I'm wondering if this is a bug with Redcon or is it possible that the PHP Redis client is expecting a string when using the ->hget() function?

What happens when you use the redis-cli command line tool?

gitmko0 commented 3 years ago
  1. I think the PHP Redis client is expecting a string when using the ->hget() function? <--- this seemed more probable.
  2. What happens when you use the redis-cli command line tool? it returns "(integer) 123456789"

but... in that case... what do u suggest for cross compatibility? how do i make it so that php redis client will return string if detection from php redis client etc?

i havent try saving a uint64 "integer" in a real redis server using go... i'm curious what's the best way forward to resolve this kind of "surprises"

tidwall commented 3 years ago

what do u suggest for cross compatibility?

For your specific HGET case. I would change

conn.WriteUint64(uint64(123456789))

to

conn.WriteString(fmt.Sprintf("%d", 123456789))

i'm curious what's the best way forward to resolve this kind of "surprises"

For best cross compatibility I recommend that you make sure your Redcon commands response types match a real Redis server response types. For example, the INCR returns an integer. So if you want to implement that command in Redcon, and you want the best compatibility across all clients, then I recommend that you return an integer.

gitmko0 commented 3 years ago

Great work! i'm looking to replace standard php wordpress redis object cache with this if mget function is written. u are good at this. my next step is waiting for mget.

what do you suggest for mget?

let's see a pseudocode... func mget(keys {}interface) { var value []byte foreach key (keys) { append(value,append(append([]byte,key...),[]byte("\n")...)...) } return value }

can you do a PR and push this code in for mget example? thx

tidwall commented 3 years ago

No. The example has enough commands to show how to use Redcon. You’ll need to implement additional commands on your own.