Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

fix LC_COLLATE issue #176 #799

Open
wants to merge 1 commit into from

3 participants

charsyam Baina Deval openbaas
charsyam

sortCompare has a bug in sort.c

if server.sort_alpha is true, sort_alpha has nothing to do with sort_bypattern.

so it has to compare with strcoll instead of compareStringObject.

and add locale config parameter to set locale easily.

charsyam charsyam referenced this pull request
Open

LC_COLLATE for SORT? #176

Baina Deval

The Redis server itself stores all data as a binary objects, so it is not dependent on the encoding. The server will just store what is sent by the client (including UTF-8 chars).

Here are a few experiments:

$ echo téléphone | hexdump -C
00000000 74 c3 a9 6c c3 a9 70 68 6f 6e 65 0a |t..l..phone.|
c3a9 is the representation of the 'é' char.

$ redis-cli

set t téléphone
OK
get t
"t\xc3\xa9l\xc3\xa9phone"
Actually the data is correctly stored in the Redis server. However, when it is launched in a terminal, the Redis client interprets the output and applies the sdscatrepr function to transform non printable chars (whose definition is locale dependent, and may be broken for multibyte chars anyway).

A simple workaround is to launch redis-cli with the 'raw' option:

$ redis-cli --raw

get t
téléphone
Your own application will probably use one of the client libraries rather than redis-cli, so it should not be a problem in practice.

charsyam

@bania. I don't think so. of course, redis server itself stores all data as a binary. but this problem is not about just storing.. for example. in Polish
redis> LPUSH test adam
(integer) 1
redis> LPUSH test łukasz
(integer) 2
redis> LPUSH test zuzanna
(integer) 3

redis> SORT test ALPHA
1) "adam"
2) "zuzanna"
3) "\xc5\x82ukasz"

łukasz has to be ahead of zuzanna.
but SORT result is wrong. we expect below result.

redis> SORT test ALPHA
1) "adam"
2) "\xc5\x82ukasz"
3) "zuzanna"

and my patch is about this problem :) Thank you.

Baina Deval

@charsyam You're right! Does your patch support redis hash sort(sorting by hash values)?
Thank you.

charsyam

@baina I don't know. I just found miss cases of sortCompare in sortCommand.

charsyam

@antirez I think this issue is obviously bug in special environment to use special letters.

charsyam charsyam referenced this pull request
Open

Sorting in redis 2.8 #1388

Jackie JackieXie168 referenced this pull request from a commit
wmrowan wmrowan fixes issue #799 9358e83
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 29, 2012
  1. openbaas

    fix LC_COLLATE issue #176

    openbaas authored
This page is out of date. Refresh to see the latest.
Showing with 32 additions and 1 deletion.
  1. +2 −0  src/config.c
  2. +7 −0 src/redis.c
  3. +2 −0  src/redis.h
  4. +21 −1 src/sort.c
2  src/config.c
View
@@ -189,6 +189,8 @@ void loadServerConfigFromString(char *config) {
if (server.maxclients < 1) {
err = "Invalid max clients limit"; goto loaderr;
}
+ } else if (!strcasecmp(argv[0],"locale") && argc == 2) {
+ server.locale = zstrdup(argv[1]);
} else if (!strcasecmp(argv[0],"maxmemory") && argc == 2) {
server.maxmemory = memtoll(argv[1],NULL);
} else if (!strcasecmp(argv[0],"maxmemory-policy") && argc == 2) {
7 src/redis.c
View
@@ -49,6 +49,8 @@
#include <math.h>
#include <sys/resource.h>
#include <sys/utsname.h>
+#include <locale.h>
+#include <stdlib.h>
/* Our shared "common" objects */
@@ -1235,6 +1237,10 @@ void initServerConfig() {
server.assert_line = 0;
server.bug_report_start = 0;
server.watchdog_period = 0;
+ server.locale = getenv("LC_COLLATE");
+ if( server.locale == NULL ){
+ server.locale = zstrdup("en_US.utf8");
+ }
}
/* This function will try to raise the max number of open files accordingly to
@@ -2672,6 +2678,7 @@ int main(int argc, char **argv) {
if (server.daemonize) createPidFile();
redisAsciiArt();
+ setlocale( LC_COLLATE, server.locale );
if (!server.sentinel_mode) {
/* Things only needed when not running in Sentinel mode. */
redisLog(REDIS_WARNING,"Server started, Redis version " REDIS_VERSION);
2  src/redis.h
View
@@ -796,6 +796,8 @@ struct redisServer {
int assert_line;
int bug_report_start; /* True if bug report header was already logged. */
int watchdog_period; /* Software watchdog period in ms. 0 = off */
+ /* locale */
+ char *locale;
};
typedef struct pubsubPattern {
22 src/sort.c
View
@@ -135,6 +135,26 @@ robj *lookupKeyByPattern(redisDb *db, robj *pattern, robj *subst) {
/* sortCompare() is used by qsort in sortCommand(). Given that qsort_r with
* the additional parameter is not standard but a BSD-specific we have to
* pass sorting parameters via the global 'server' structure */
+int compareStringWithLocale(robj *a, robj *b) {
+ redisAssertWithInfo(NULL,a,a->type == REDIS_STRING && b->type == REDIS_STRING);
+ char bufa[128], bufb[128], *astr, *bstr;
+
+ if (a == b) return 0;
+ if (a->encoding != REDIS_ENCODING_RAW) {
+ ll2string(bufa,sizeof(bufa),(long) a->ptr);
+ astr = bufa;
+ } else {
+ astr = a->ptr;
+ }
+ if (b->encoding != REDIS_ENCODING_RAW) {
+ ll2string(bufb,sizeof(bufb),(long) b->ptr);
+ bstr = bufb;
+ } else {
+ bstr = b->ptr;
+ }
+ return strcoll(astr,bstr);
+}
+
int sortCompare(const void *s1, const void *s2) {
const redisSortObject *so1 = s1, *so2 = s2;
int cmp;
@@ -168,7 +188,7 @@ int sortCompare(const void *s1, const void *s2) {
}
} else {
/* Compare elements directly. */
- cmp = compareStringObjects(so1->obj,so2->obj);
+ cmp = compareStringWithLocale(so1->obj,so2->obj);
}
}
return server.sort_desc ? -cmp : cmp;
Something went wrong with that request. Please try again.