Skip to content

Commit

Permalink
Merge pull request #1183 from lucifercr07/lucifercr07/fix_hll_respons…
Browse files Browse the repository at this point in the history
…e_types_for_wrong_key

Refactor Hyperloglog wrong type responses and removed logger references.
  • Loading branch information
arpitbbhayani authored Oct 22, 2024
2 parents 92ac20e + 6b234f6 commit bf25a7a
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 143 deletions.
16 changes: 8 additions & 8 deletions docs/src/content/docs/commands/PFADD.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
title: PFADD
title: PFADD
description: The `PFADD` command in DiceDB is used to add elements to a HyperLogLog data structure. HyperLogLog is a probabilistic data structure used for estimating the cardinality of a set, i.e., the number of unique elements in a dataset.
---

Expand Down Expand Up @@ -37,17 +37,17 @@ PFADD key element [element ...]

1. `Wrong type error`:

- Error Message: `(error) ERROR WRONGTYPE Key is not a valid HyperLogLog string value.`
- Error Message: `(error) WRONGTYPE Key is not a valid HyperLogLog string value`
- Occurs when trying to use the command on a key that is not a HyperLogLog.

2. `Syntax error`:

- Error Message: `(error) ERROR wrong number of arguments for 'pfadd' command`
- Error Message: `(error) wrong number of arguments for 'pfadd' command`
- Occurs when the command syntax is incorrect or missing required parameters.

## Examples
## Example Usage

### Basic Example
### Basic Usage

Adding a single element to a HyperLogLog:

Expand All @@ -74,15 +74,15 @@ If the elements do not alter the internal registers:
(integer) 0
```

### Error Example
### Invalid Usage

Attempting to add elements to a key that is not a HyperLogLog:

```bash
127.0.0.1:7379> SET mykey "notahyperloglog"
OK
127.0.0.1:7379> PFADD mykey "element1"
(error) ERROR WRONGTYPE Key is not a valid HyperLogLog string value.
(error) WRONGTYPE Key is not a valid HyperLogLog string value
```

---
---
5 changes: 3 additions & 2 deletions docs/src/content/docs/commands/PFCOUNT.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ The `PFCOUNT` command can raise errors in the following scenarios:
(error) ERR wrong number of arguments for 'pfcount' command
```

## Examples
## Example usage

### Basic Usage

### Single Key

Expand Down Expand Up @@ -91,4 +93,3 @@ The `PFCOUNT` command can raise errors in the following scenarios:
- HyperLogLog is particularly useful for large datasets where an exact count is not feasible due to memory constraints.

By understanding the `PFCOUNT` command, you can efficiently estimate the cardinality of large sets with minimal memory usage, making it a powerful tool for various applications such as analytics and monitoring.

6 changes: 3 additions & 3 deletions docs/src/content/docs/commands/PFMERGE.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ PFMERGE destkey sourcekey [sourcekey ...]
- The command retrieves the HyperLogLog data structures from the specified `sourcekey` keys.
- These `sourcekey` keys are merged into a single HyperLogLog and stored in the `destkey`.
- If the `sourcekey` keys are not valid HyperLogLogs, an error is returned.


## Errors

Expand Down Expand Up @@ -95,6 +95,6 @@ if a `sourcekey` exists and is not of type HyperLogLog, the command will result

```sh
127.0.0.1:7379> PFMERGE hll_merged not_hyperLogLog
(error) WRONGTYPE Key is not a valid HyperLogLog string value.
(error) WRONGTYPE Key is not a valid HyperLogLog string value

```
```
97 changes: 0 additions & 97 deletions integration_tests/commands/async/hyperloglog_test.go

This file was deleted.

4 changes: 2 additions & 2 deletions integration_tests/commands/http/hyperloglog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func TestHyperLogLogCommands(t *testing.T) {
{Command: "SET", Body: map[string]interface{}{"key": "INVALID_HLL", "value": "1"}},
{Command: "PFMERGE", Body: map[string]interface{}{"key": "INVALID_HLL"}},
},
expected: []interface{}{float64(1), "OK", "ERR -WRONGTYPE Key is not a valid HyperLogLog string value."},
expected: []interface{}{float64(1), "OK", "WRONGTYPE Key is not a valid HyperLogLog string value"},
delays: []time.Duration{0, 0, 0},
},
{
Expand All @@ -126,7 +126,7 @@ func TestHyperLogLogCommands(t *testing.T) {
{Command: "SET", Body: map[string]interface{}{"key": "INVALID_SRC_HLL", "value": "1"}},
{Command: "PFMERGE", Body: map[string]interface{}{"keys": [...]string{"HLL", "INVALID_SRC_HLL"}}},
},
expected: []interface{}{float64(1), "OK", "ERR -WRONGTYPE Key is not a valid HyperLogLog string value."},
expected: []interface{}{float64(1), "OK", "WRONGTYPE Key is not a valid HyperLogLog string value"},
delays: []time.Duration{0, 0, 0},
},
}
Expand Down
4 changes: 1 addition & 3 deletions integration_tests/commands/http/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"errors"
"fmt"
"log"
"log/slog"
"net/http"
"strings"
"sync"
Expand All @@ -24,8 +23,7 @@ import (
)

type TestServerOptions struct {
Port int
Logger *slog.Logger
Port int
}

type CommandExecutor interface {
Expand Down
10 changes: 7 additions & 3 deletions integration_tests/commands/resp/hyperloglog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ func TestHyperLogLogCommands(t *testing.T) {
conn := getLocalConnection()
defer conn.Close()

testCases := []TestCase{
testCases := []struct {
name string
commands []string
expected []interface{}
}{
{
name: "PFADD with one key-value pair",
commands: []string{"PFADD hll0 v1", "PFCOUNT hll0"},
Expand Down Expand Up @@ -72,13 +76,13 @@ func TestHyperLogLogCommands(t *testing.T) {
name: "PFMERGE with invalid object",
commands: []string{
"PFADD INVALID_HLL a b c", "SET INVALID_HLL \"1\"", "PFMERGE INVALID_HLL"},
expected: []interface{}{int64(1), "OK", "ERR -WRONGTYPE Key is not a valid HyperLogLog string value."},
expected: []interface{}{int64(1), "OK", "WRONGTYPE Key is not a valid HyperLogLog string value"},
},
{
name: "PFMERGE with invalid src object",
commands: []string{
"PFADD INVALID_SRC_HLL a b c", "SET INVALID_SRC_HLL \"1\"", "PFMERGE HLL INVALID_SRC_HLL"},
expected: []interface{}{int64(1), "OK", "ERR -WRONGTYPE Key is not a valid HyperLogLog string value."},
expected: []interface{}{int64(1), "OK", "WRONGTYPE Key is not a valid HyperLogLog string value"},
},
}

Expand Down
3 changes: 1 addition & 2 deletions integration_tests/commands/resp/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ import (
)

type TestServerOptions struct {
Port int
Logger *slog.Logger
Port int
}

//nolint:unused
Expand Down
4 changes: 2 additions & 2 deletions integration_tests/commands/websocket/hyperloglog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,13 @@ func TestHyperLogLogCommands(t *testing.T) {
name: "PFMERGE with invalid object",
commands: []string{
"PFADD INVALID_HLL a b c", "SET INVALID_HLL \"1\"", "PFMERGE INVALID_HLL"},
expected: []interface{}{float64(1), "OK", "ERR -WRONGTYPE Key is not a valid HyperLogLog string value."},
expected: []interface{}{float64(1), "OK", "WRONGTYPE Key is not a valid HyperLogLog string value"},
},
{
name: "PFMERGE with invalid src object",
commands: []string{
"PFADD INVALID_SRC_HLL a b c", "SET INVALID_SRC_HLL \"1\"", "PFMERGE HLL INVALID_SRC_HLL"},
expected: []interface{}{float64(1), "OK", "ERR -WRONGTYPE Key is not a valid HyperLogLog string value."},
expected: []interface{}{float64(1), "OK", "WRONGTYPE Key is not a valid HyperLogLog string value"},
},
}

Expand Down
3 changes: 1 addition & 2 deletions integration_tests/commands/websocket/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ const (
)

type TestServerOptions struct {
Port int
Logger *slog.Logger
Port int
}

type CommandExecutor interface {
Expand Down
6 changes: 3 additions & 3 deletions internal/eval/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2288,10 +2288,10 @@ func testEvalPFADD(t *testing.T, store *dstore.Store) {
store.Put(key, store.NewObj(value, exDurationMs, oType, oEnc), dstore.WithKeepTTL(keepttl))
},
input: []string{"EXISTING_KEY", "1"},
output: []byte("-WRONGTYPE Key is not a valid HyperLogLog string value."),
output: []byte("-WRONGTYPE Key is not a valid HyperLogLog string value"),
migratedOutput: EvalResponse{
Result: nil,
Error: diceerrors.ErrGeneral("-WRONGTYPE Key is not a valid HyperLogLog string value."),
Error: diceerrors.ErrInvalidHyperLogLogKey,
},
},
}
Expand Down Expand Up @@ -2376,7 +2376,7 @@ func testEvalPFMERGE(t *testing.T, store *dstore.Store) {
input: []string{"INVALID_OBJ_DEST_KEY"},
migratedOutput: EvalResponse{
Result: nil,
Error: diceerrors.ErrGeneral("-WRONGTYPE Key is not a valid HyperLogLog string value."),
Error: diceerrors.ErrInvalidHyperLogLogKey,
},
},
"PFMERGE destKey doesn't exist": {
Expand Down
12 changes: 6 additions & 6 deletions internal/eval/store_eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,7 @@ func evalPFADD(args []string, store *dstore.Store) *EvalResponse {
if !ok {
return &EvalResponse{
Result: nil,
Error: diceerrors.ErrGeneral(diceerrors.WrongTypeHllErr),
Error: diceerrors.ErrInvalidHyperLogLogKey,
}
}
initialCardinality := existingHll.Estimate()
Expand Down Expand Up @@ -973,14 +973,14 @@ func evalPFCOUNT(args []string, store *dstore.Store) *EvalResponse {
if !ok {
return &EvalResponse{
Result: nil,
Error: diceerrors.ErrGeneral(diceerrors.WrongTypeHllErr),
Error: diceerrors.ErrInvalidHyperLogLogKey,
}
}
err := unionHll.Merge(currKeyHll)
if err != nil {
return &EvalResponse{
Result: nil,
Error: diceerrors.ErrGeneral(diceerrors.InvalidHllErr),
Error: diceerrors.ErrCorruptedHyperLogLogObject,
}
}
}
Expand Down Expand Up @@ -1130,7 +1130,7 @@ func evalPFMERGE(args []string, store *dstore.Store) *EvalResponse {
if !ok {
return &EvalResponse{
Result: nil,
Error: diceerrors.ErrGeneral(diceerrors.WrongTypeHllErr),
Error: diceerrors.ErrInvalidHyperLogLogKey,
}
}
}
Expand All @@ -1142,15 +1142,15 @@ func evalPFMERGE(args []string, store *dstore.Store) *EvalResponse {
if !ok {
return &EvalResponse{
Result: nil,
Error: diceerrors.ErrGeneral(diceerrors.WrongTypeHllErr),
Error: diceerrors.ErrInvalidHyperLogLogKey,
}
}

err := mergedHll.Merge(currKeyHll)
if err != nil {
return &EvalResponse{
Result: nil,
Error: diceerrors.ErrGeneral(diceerrors.InvalidHllErr),
Error: diceerrors.ErrCorruptedHyperLogLogObject,
}
}
}
Expand Down
Loading

0 comments on commit bf25a7a

Please sign in to comment.