Encoding error on GET for keys created using SETBIT

[soumya-codes]

Trying to perform a GET operation on a key that was created using the SETBIT command results in encoding error.

How to reproduce

192.168.1.3:7379> SETBIT mykey369 7 1
(integer) 0
192.168.1.3:7379> GET mykey369
(error) ERR unsupported encoding: 4

Expected behavior GET on key created using SETBIT should return correct value.

Corresponding output from REDIS

redis:6379> SETBIT mykey369 7 1
(integer) 0
redis:6379> GET mykey369
"\x01"

Screenshots

[kushal0511-not]

@soumya-codes i think it is issue in evalGET , we have not added case for "ObjEncodingByteArray" in typeEnconding. i tried with adding that in evalGet add got expected result. possible patch in evalGET:

	case ObjEncodingByteArray:
		// Value is stored as a bytearray, use type assertion
		if val, ok := obj.Value.(*ByteArray); ok {
			return Encode(string(val.data), false)
		}
		return Encode(errors.New("ERR expected byte array but got another type"), false)

after applying patch

cc @JyotinderSingh

[JyotinderSingh]

@soumya-codes i think it is issue in evalGET , we have not added case for "ObjEncodingByteArray" in typeEnconding.

i tried with adding that in evalGet add got expected result.

possible patch in evalGET:

	case ObjEncodingByteArray:

		// Value is stored as a bytearray, use type assertion

		if val, ok := obj.Value.(*ByteArray); ok {

			return Encode(string(val.data), false)

		}

		return Encode(errors.New("ERR expected byte array but got another type"), false)

after applying patch

cc @JyotinderSingh

Thanks for catching this bug! Could you raise a PR for the same? Please add a test for this case too.

[kushal0511-not]

Thanks for catching this bug! Could you raise a PR for the same? Please add a test for this case too.

Sure

[lucifercr07]

+1 this case also happens with other commands

0.0.0.0:7379> LPUSH mykey "value1"
OK
0.0.0.0:7379> GET mykey
(error) ERR unsupported encoding: 4

[kushal0511-not]

@JyotinderSingh also we are not handling case where existing key has value integer type. we need to change condition here.(https://github.com/DiceDB/dice/blob/master/core/eval.go#L1409)

current behavior

expected behavior

i will fix this also in same PR.

[JyotinderSingh]

@JyotinderSingh also we are not handling case where existing key has value integer type.

we need to add here.(https://github.com/DiceDB/dice/blob/master/core/eval.go#L1409)

current behavior

expected behavior

i will fix this also in same PR.

Thank you!

[soumya-codes]

@JyotinderSingh @kushal0511-not @lucifercr07 I think we need a broader discussion on encoding type. I have few things in mind that we need to look into.

1. Is it correct for SETBIT to use "ObjEncodingByteArray" while storing the values. Why can't we use string encoding like we do for SET?
2. What should be the supported encoding types and under what scenarios should we use each type of encoding.

Here are the list of supported encoding types in DiceDB vs Redis:

REDIS Object Types

#define OBJ_STRING 0 /* String object. / #define OBJ_LIST 1 / List object. / #define OBJ_SET 2 / Set object. / #define OBJ_ZSET 3 / Sorted set object. / #define OBJ_HASH 4 / Hash object. */

Object Encoding supported in REDIS

/* Objects encoding. Some kind of objects like Strings and Hashes can be

• internally represented in multiple ways. The 'encoding' field of the object
• is set to one of this fields for this object. / #define OBJ_ENCODING_RAW 0 / Raw representation / #define OBJ_ENCODING_INT 1 / Encoded as integer / #define OBJ_ENCODING_HT 2 / Encoded as hash table / #define OBJ_ENCODING_ZIPMAP 3 / No longer used: old hash encoding. / #define OBJ_ENCODING_LINKEDLIST 4 / No longer used: old list encoding. / #define OBJ_ENCODING_ZIPLIST 5 / No longer used: old list/hash/zset encoding. / #define OBJ_ENCODING_INTSET 6 / Encoded as intset / #define OBJ_ENCODING_SKIPLIST 7 / Encoded as skiplist / #define OBJ_ENCODING_EMBSTR 8 / Embedded sds string encoding / #define OBJ_ENCODING_QUICKLIST 9 / Encoded as linked list of listpacks / #define OBJ_ENCODING_STREAM 10 / Encoded as a radix tree of listpacks / #define OBJ_ENCODING_LISTPACK 11 / Encoded as a listpack / #define OBJ_ENCODING_LISTPACK_EX 12 / Encoded as listpack, extended with metadata */

Dice DB Object Types

var ObjTypeString uint8 = 0 << 4 var ObjTypeByteList uint8 = 1 << 4 var ObjTypeJSON uint8 = 3 << 4 // 00110000 var ObjTypeInt uint8 = 5 << 4 // 01010000 var ObjTypeBitSet uint8 = 2 << 4 // 00100000 var ObjTypeByteArray uint8 = 4 << 4 // 01000000

Dice DB Object Encoding

var ObjEncodingRaw uint8 = 0 var ObjEncodingInt uint8 = 1 var ObjEncodingEmbStr uint8 = 8 var ObjEncodingQint uint8 = 0 var ObjEncodingQref uint8 = 1 var ObjEncodingStackInt uint8 = 2 var ObjEncodingStackRef uint8 = 3 var ObjEncodingDeque uint8 = 4 var ObjEncodingBF uint8 = 2 // 00000010 var ObjEncodingJSON uint8 = 0 var ObjEncodingByteArray uint8 = 4

As can be seen we differ on the Object and ObjectEncoding types when compared to REDIS. Lets make sure we are following the correct approach while handling encoding.

cc @AshwinKul28

Let me know what you all think about this.

[JyotinderSingh]

@JyotinderSingh @kushal0511-not @lucifercr07 I think we need a broader discussion on encoding type.

I have few things in mind that we need to look into.

1. Is it correct for SETBIT to use "ObjEncodingByteArray" while storing the values. Why can't we use string encoding like we do for SET?

2. What should be the supported encoding types and under what scenarios should we use each type of encoding.

Here are the list of supported encoding types in DiceDB vs Redis:

REDIS Object Types

#define OBJ_STRING 0 /* String object. */

#define OBJ_LIST 1 /* List object. */

#define OBJ_SET 2 /* Set object. */

#define OBJ_ZSET 3 /* Sorted set object. */

#define OBJ_HASH 4 /* Hash object. */

Object Encoding supported in REDIS

/* Objects encoding. Some kind of objects like Strings and Hashes can be

• internally represented in multiple ways. The 'encoding' field of the object

• is set to one of this fields for this object. */

#define OBJ_ENCODING_RAW 0 /* Raw representation */

#define OBJ_ENCODING_INT 1 /* Encoded as integer */

#define OBJ_ENCODING_HT 2 /* Encoded as hash table */

#define OBJ_ENCODING_ZIPMAP 3 /* No longer used: old hash encoding. */

#define OBJ_ENCODING_LINKEDLIST 4 /* No longer used: old list encoding. */

#define OBJ_ENCODING_ZIPLIST 5 /* No longer used: old list/hash/zset encoding. */

#define OBJ_ENCODING_INTSET 6 /* Encoded as intset */

#define OBJ_ENCODING_SKIPLIST 7 /* Encoded as skiplist */

#define OBJ_ENCODING_EMBSTR 8 /* Embedded sds string encoding */

#define OBJ_ENCODING_QUICKLIST 9 /* Encoded as linked list of listpacks */

#define OBJ_ENCODING_STREAM 10 /* Encoded as a radix tree of listpacks */

#define OBJ_ENCODING_LISTPACK 11 /* Encoded as a listpack */

#define OBJ_ENCODING_LISTPACK_EX 12 /* Encoded as listpack, extended with metadata */

Dice DB Object Types

var ObjTypeString uint8 = 0 << 4

var ObjTypeByteList uint8 = 1 << 4

var ObjTypeJSON uint8 = 3 << 4 // 00110000

var ObjTypeInt uint8 = 5 << 4 // 01010000

var ObjTypeBitSet uint8 = 2 << 4 // 00100000

var ObjTypeByteArray uint8 = 4 << 4 // 01000000

Dice DB Object Encoding

var ObjEncodingRaw uint8 = 0

var ObjEncodingInt uint8 = 1

var ObjEncodingEmbStr uint8 = 8

var ObjEncodingQint uint8 = 0

var ObjEncodingQref uint8 = 1

var ObjEncodingStackInt uint8 = 2

var ObjEncodingStackRef uint8 = 3

var ObjEncodingDeque uint8 = 4

var ObjEncodingBF uint8 = 2 // 00000010

var ObjEncodingJSON uint8 = 0

var ObjEncodingByteArray uint8 = 4

As can be seen we differ on the Object and ObjectEncoding types when compared to REDIS.

Lets make sure we are following the correct approach while handling encoding.

cc @AshwinKul28

Let me know what you all think about this.

Let's discuss this in today's call?