DiceDB / dice

DiceDB is a redis-compliant, reactive, scalable, highly-available, unified cache optimized for modern hardware.
https://dicedb.io/
Other
6.88k stars 1.1k forks source link

Inconsistent JSON.STRAPPEND: The return type of JSON.STRAPPEND should be integer but it is returning string #1108

Closed shashi-sah2003 closed 1 month ago

shashi-sah2003 commented 1 month ago

Steps to reproduce

  1. JSON.SET doc $ '{"a":"foo", "nested": {"a": "hello"}, "nested2": {"a": 31}}'
  2. JSON.STRAPPEND doc $..a '"baz"'

Expected output

image

The expected output when the above set of commands when run on Redis

Observed output

image

The observed output when the above set of commands when run on DiceDB

The steps to run the test cases are mentioned in the README of the dice-tests repository.

Expectations for resolution

This issue will be considered resolved when the following things are done

  1. changes in the dice code to meet the expected behavior
  2. Successful run of the tcl test behavior

You can find the tests under the tests directory of the dice repository and the steps to run are in the README file. Refer to the following links to set up DiceDB and Redis 7.2.5 locally

shashi-sah2003 commented 1 month ago

@JyotinderSingh @lucifercr07 I would like to work on this issue, pls assign it to me thanks

surya0180 commented 1 month ago

@shashi-sah2003 Can you tag me here when you solve this issue? Because I was really frustrated no matter how much time I spend on this issue when I was working on JSON.ARRLEN. Please have a look at this comment in this issue, This user says that the output is coming correctly in MacOS, but in windows, it is coming differently

https://github.com/DiceDB/dice/pull/976#issuecomment-2395457327

iamskp11 commented 1 month ago

@shashi-sah2003 Can you tag me here when you solve this issue? Because I was really frustrated no matter how much time I spend on this issue when I was working on JSON.ARRLEN. Please have a look at this comment in this issue, This user says that the output is coming correctly in MacOS, but in windows, it is coming differently

https://github.com/DiceDB/dice/pull/976#issuecomment-2395457327

Hey @surya0180 , did you verify if this is Windows specific issue?

surya0180 commented 1 month ago

@shashi-sah2003 Can you tag me here when you solve this issue? Because I was really frustrated no matter how much time I spend on this issue when I was working on JSON.ARRLEN. Please have a look at this comment in this issue, This user says that the output is coming correctly in MacOS, but in windows, it is coming differently #976 (comment)

Hey @surya0180 , did you verify if this is Windows specific issue?

@iamskp11 From people I know who are using windows, all are facing the same issue. Though I did not check with people who are using mac in particular, I still think its something related to the terminal used in the OS

shashi-sah2003 commented 1 month ago

@surya0180 I am going to work on this issue today. btw have you confirmed if this issue is only windows specific?

shashi-sah2003 commented 1 month ago

@JyotinderSingh @lucifercr07 can you guys pls look at this issue as I can see in this function integer type is being appended in the resultArray but it's returning this https://github.com/DiceDB/dice/issues/1108#issue-2590724286

func evalJSONSTRAPPEND(args []string, store *dstore.Store) []byte {
    if len(args) != 3 {
        return diceerrors.NewErrArity("JSON.STRAPPEND")
    }

    key := args[0]
    path := args[1]
    value := args[2]

    obj := store.Get(key)
    if obj == nil {
        return diceerrors.NewErrWithMessage(diceerrors.NoKeyExistsErr)
    }

    errWithMessage := object.AssertTypeAndEncoding(obj.TypeEncoding, object.ObjTypeJSON, object.ObjEncodingJSON)
    if errWithMessage != nil {
        return diceerrors.NewErrWithFormattedMessage(diceerrors.WrongKeyTypeErr)
    }

    jsonData := obj.Value

    var resultsArray []interface{}

    if path == "$" {
        // Handle root-level string
        if str, ok := jsonData.(string); ok {
            unquotedValue := strings.Trim(value, "\"")
            newValue := str + unquotedValue
            resultsArray = append(resultsArray, int64(len(newValue)))
            jsonData = newValue
        } else {
            return clientio.RespEmptyArray
        }
    } else {
        expr, err := jp.ParseString(path)
        if err != nil {
            return clientio.RespEmptyArray
        }

        _, modifyErr := expr.Modify(jsonData, func(data any) (interface{}, bool) {
            switch v := data.(type) {
            case string:
                unquotedValue := strings.Trim(value, "\"")
                newValue := v + unquotedValue
                resultsArray = append([]interface{}{int64(len(newValue))}, resultsArray...)
                return newValue, true
            default:
                resultsArray = append([]interface{}{clientio.RespNIL}, resultsArray...)
                return data, false
            }
        })

        if modifyErr != nil {
            return clientio.RespEmptyArray
        }
    }

    if len(resultsArray) == 0 {
        return clientio.RespEmptyArray
    }

    obj.Value = jsonData
    return clientio.Encode(resultsArray, false)
}
JyotinderSingh commented 1 month ago

@JyotinderSingh @lucifercr07 can you guys pls look at this issue as I can see in this function integer type is being appended in the resultArray but it's returning this #1108 (comment)

func evalJSONSTRAPPEND(args []string, store *dstore.Store) []byte {
  if len(args) != 3 {
      return diceerrors.NewErrArity("JSON.STRAPPEND")
  }

  key := args[0]
  path := args[1]
  value := args[2]

  obj := store.Get(key)
  if obj == nil {
      return diceerrors.NewErrWithMessage(diceerrors.NoKeyExistsErr)
  }

  errWithMessage := object.AssertTypeAndEncoding(obj.TypeEncoding, object.ObjTypeJSON, object.ObjEncodingJSON)
  if errWithMessage != nil {
      return diceerrors.NewErrWithFormattedMessage(diceerrors.WrongKeyTypeErr)
  }

  jsonData := obj.Value

  var resultsArray []interface{}

  if path == "$" {
      // Handle root-level string
      if str, ok := jsonData.(string); ok {
          unquotedValue := strings.Trim(value, "\"")
          newValue := str + unquotedValue
          resultsArray = append(resultsArray, int64(len(newValue)))
          jsonData = newValue
      } else {
          return clientio.RespEmptyArray
      }
  } else {
      expr, err := jp.ParseString(path)
      if err != nil {
          return clientio.RespEmptyArray
      }

      _, modifyErr := expr.Modify(jsonData, func(data any) (interface{}, bool) {
          switch v := data.(type) {
          case string:
              unquotedValue := strings.Trim(value, "\"")
              newValue := v + unquotedValue
              resultsArray = append([]interface{}{int64(len(newValue))}, resultsArray...)
              return newValue, true
          default:
              resultsArray = append([]interface{}{clientio.RespNIL}, resultsArray...)
              return data, false
          }
      })

      if modifyErr != nil {
          return clientio.RespEmptyArray
      }
  }

  if len(resultsArray) == 0 {
      return clientio.RespEmptyArray
  }

  obj.Value = jsonData
  return clientio.Encode(resultsArray, false)
}

My guess here is that we may be encoding it the wrong way, the best way to debug this is to attach a debugger.

JyotinderSingh commented 1 month ago

I just tried this on my local (with both async and resp servers) and I am unable to reproduce the issue

> redis-cli -p 7379                                            
127.0.0.1:7379> JSON.SET doc $ '{"a":"foo", "nested": {"a": "hello"}, "nested2": {"a": 31}}'
OK
127.0.0.1:7379> JSON.STRAPPEND doc $..a '"baz"'
1) (integer) 6
2) (integer) 8
3) (nil)
127.0.0.1:7379> 
JyotinderSingh commented 1 month ago

Screenshot from 2024-10-20 12-59-22

shashi-sah2003 commented 1 month ago

@JyotinderSingh I tried your method of accessing dicedb server using redis-cli, it's working correctly as you have shown above but while accessing it through dicedb-cli it's producing the issue that I have mentioned above. image image

Is this some kind of bug with dicedb-cli?

JyotinderSingh commented 1 month ago

@JyotinderSingh I tried your method of accessing dicedb server using redis-cli, it's working correctly as you have shown above but while accessing it through dicedb-cli it's producing the issue that I have mentioned above.

image

image

Is this some kind of bug with dicedb-cli?

It might be a bug with the CLI. We'll need to look into it a little deeper.

shashi-sah2003 commented 1 month ago

@JyotinderSingh as this is related to bug with the CLI, should I close this issue?