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

Add support of RESP3 attribute type #1248

Merged
merged 2 commits into from
Jan 31, 2024

Conversation

git-hulk
Copy link
Contributor

Currently, Redis DEBUG PROTOCOL 'attrib' command will return an attribute type, but hiredis doesn't support it yet. So it got the protocol type error:

127.0.0.1:6379>  DEBUG PROTOCOL attrib
Error: Protocol error, got "|" as reply type byte

After apply this PR, it should reply:

127.0.0.1:6379> DEBUG PROTOCOL attrib
1# "key-popularity"
1# 1) "key:123"
   2) (integer) 90

Currently, Redis DEBUG PROTOCOL 'attrib' command will return an
attribute type, but hiredis doesn't support it yet. So it got the
protocol type error:

```
127.0.0.1:6379>  DEBUG PROTOCOL attrib
Error: Protocol error, got "|" as reply type byte
```

After apply this PR, it should reply:

```
127.0.0.1:6379> DEBUG PROTOCOL attrib
1# "key-popularity"
1# 1) "key:123"
   2) (integer) 90
```
@git-hulk
Copy link
Contributor Author

redis-cli also doesn't support the RESP3 attribute type yet. I can submit a PR after this patch is merged and updated in redis repository.

Copy link
Collaborator

@michael-grunder michael-grunder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you wnat to make the minor formatting fixes I'll get this merged.

hiredis.c Show resolved Hide resolved
read.c Outdated Show resolved Hide resolved
@git-hulk
Copy link
Contributor Author

Hi @michael-grunder Thanks for your review. All review comments are resolved, please retake a look while you're free, thank you!

@michael-grunder michael-grunder merged commit ab30060 into redis:master Jan 31, 2024
12 checks passed
@michael-grunder
Copy link
Collaborator

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants