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.

Sign up for GitHub

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

Jump to bottom

Pubsub numsub -> counts should be integers, not strings. #1561

Closed
jonathanslenders opened this issue Feb 23, 2014 · 2 comments
Closed

Pubsub numsub -> counts should be integers, not strings. #1561

jonathanslenders opened this issue Feb 23, 2014 · 2 comments

Comments

@jonathanslenders
Copy link

Right now, the "pubsub numsub" command returns the counts as a string. I think this is wrong. We have integer data types in our protocol.

Current output:

> telnet localhost 6379
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
pubsub numsub channel1 channel2
*4
$8
channel1
$1
0
$8
channel2
$1
0

Expected:

pubsub numsub channel1 channel2
*4
$8
channel1
:0
$8
channel2
:0
@jonathanslenders
Copy link
Author

By the way, integers in multibulk are used elsewhere. E.g. in "script exists":

script exists aaa bbb ccc
*3
:0
:0
:0

mattsta added a commit to mattsta/redis that referenced this issue Aug 2, 2014
@mattsta
pubsub: Return integers for NUMSUB, not strings
95e182d 
Also adds test for numsub — due to tcl being tcl,
it doesn't capture the "numberness" of the fix,
but now we at least have one test case for numsub.

Closes redis#1561
mattsta added a commit to mattsta/redis that referenced this issue Aug 2, 2014
@mattsta
pubsub: Return integers for NUMSUB, not strings
b1d224a 
Also adds test for numsub — due to tcl being tcl,
it doesn't capture the "numberness" of the fix,
but now we at least have one test case for numsub.

Closes redis#1561
@mattsta mattsta mentioned this issue Aug 2, 2014
Closed
mattsta added a commit to mattsta/redis that referenced this issue Aug 6, 2014
@mattsta
pubsub: Return integers for NUMSUB, not strings
3c100be 
Also adds test for numsub — due to tcl being tcl,
it doesn't capture the "numberness" of the fix,
but now we at least have one test case for numsub.

Closes redis#1561
mattsta added a commit that referenced this issue Aug 26, 2014
@mattsta @antirez
pubsub: Return integers for NUMSUB, not strings
961e5e2 
Also adds test for numsub — due to tcl being tcl,
it doesn't capture the "numberness" of the fix,
but now we at least have one test case for numsub.

Closes #1561
@HeartSaVioR HeartSaVioR mentioned this issue Aug 26, 2014
Open
@HeartSaVioR
Copy link
Contributor

@mattsta It could break client's response parser which should support multi-versions of Redis.
(especially strong type language)
So it should be documented, and notified well.

@HeartSaVioR HeartSaVioR mentioned this issue Aug 26, 2014
Closed
mattsta added a commit that referenced this issue Aug 27, 2014
@mattsta @antirez
pubsub: Return integers for NUMSUB, not strings
09a3976 
Also adds test for numsub — due to tcl being tcl,
it doesn't capture the "numberness" of the fix,
but now we at least have one test case for numsub.

Closes #1561
@dim dim mentioned this issue Jan 15, 2015
Closed
@slact slact mentioned this issue Feb 29, 2016
Closed
@darrachequesne darrachequesne mentioned this issue Nov 29, 2016
Merged
@moria7757 moria7757 mentioned this issue Dec 29, 2020
Closed
@qq1052121189 qq1052121189 mentioned this issue Feb 7, 2021
Closed
@stefanschindler stefanschindler mentioned this issue Apr 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jonathanslenders @HeartSaVioR