[tarantool-patches] Re: [PATCH v2 2/3] Introduce "none" and "binary" collations

["n.pettik" <korablev@tarantool.org>]

> On 14 Nov 2018, at 14:52, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
>>>> 
>>>> +	/*
>>>> +	 * In SQL all identifiers should be uppercased, so
>>>> +	 * to avoid mess lets simple search binary (since it is
>>>> +	 * sort of "special" collation) ignoring case at all.
>>>> +	 */
>>> 
>>> 2. I am not sure if it should be so special. I think, we should
>>> treat it just like any other collation. If tests have BINARY
>>> uppercased, then they should be fixed, I guess.
>> Thing is in SQLite binary can be used without quotes, like:
>> … COLLATE BiNaRy …
>> And since ids are uppercased, this thing comes as “BINARY".
>> If I forced only lower case “binary”, a lot (>80 separate sub-tests,
>> 12 separate file tests) of tests would fail. Moreover, I see that similar
>> things take place at Lua-land (schema.cc):
>> local function update_index_parts(format, parts)
>> ...
>> elseif k == 'collation' then
>>     -- find ID by name
>>     local coll = box.space._collation.index.name:get{v}
>>     if not coll then
>>         coll = box.space._collation.index.name:get{v:lower()}
>>     end
>> If you still think that we should allow only “binary” format,
>> I will do it alongside with tests in a separate commit.
> 
> I still think that since 'binary' is even in _collation space
> now, it should be just a regular collation. If you do not want
> to change tests, then you can try to search any collation firstly
> as is, and if not found, then lowerify and search again, regardless
> of is it binary or not.

We can’t lower each collation’s name - I’m sure that Peter will
be strictly against this change (to be honest I am too), because
it contradicts our rules concerning identifications’ names.
Moreover, what if user create two collations with names like
“custom_collation” and “CuStom_COLLATION” and execute query like

SELECT … COLLATE “custom_COLLATION"

Which collation will be used in this case? Without lowering no one
is found, but after lowering both would be available.
I would better fix tests to make them use “binary” instead of
BINARY. I guess this is absolutely right solution.
Commit at the end of letter.

> My point is that we should not treat binary differently from
> others.

Yep, now I fully agree with you. I am going to add one more
commit to patchset.

> By the way, what if I write SQL like this? :
> 
>    SELECT ... WHERE  ... COLLATE NONE …
> 
> Looks like this code:
> 
>> +       if (strcasecmp(name, "binary") == 0)
>> +               p = coll_by_name("binary", strlen("binary"));
>> +       else
>> +               p = coll_by_name(name, strlen(name));
> 
> will find it ok, but maybe should not, is it?

Why? It exists in _collation and uses memcmp, so what is wrong with it? 
I guess we simply should document its behaviour.
Also, in your example parser would fail to find it. Correct usage is like:

> SELECT ... WHERE  ... COLLATE “none” …

:)

On the contrary, I've added tests on this case:

diff --git a/test/sql-tap/identifier_case.test.lua b/test/sql-tap/identifier_case.test.lua
index 4878ced65..9e8107aef 100755
--- a/test/sql-tap/identifier_case.test.lua
+++ b/test/sql-tap/identifier_case.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(67)
+test:plan(71)
 
 local test_prefix = "identifier_case-"
 
@@ -207,7 +207,9 @@ data = {
     { 4,  [["bInaRy"]], {1, "Collation 'bInaRy' does not exist"}},
     { 5,  [["unicode"]], {0}},
     { 6,  [[ unicode ]], {1,"Collation 'UNICODE' does not exist"}},
-    { 7,  [["UNICODE"]], {1,"Collation 'UNICODE' does not exist"}}
+    { 7,  [["UNICODE"]], {1,"Collation 'UNICODE' does not exist"}},
+    { 8,  [[NONE]], {1,"Collation 'NONE' does not exist"}},
+    { 9,  [["none"]], {0}},
 }
 
 test:do_catchsql_test(
@@ -241,6 +243,8 @@ data = {
     { 5,  [[ 5 < 'b' collate "unicode" ]], {0, {1}}},
     { 6,  [[ 5 < 'b' collate unicode ]], {1,"Collation 'UNICODE' does not exist"}},
     { 7,  [[ 5 < 'b' collate "unicode_ci" ]], {0, {1}}},
+    { 8,  [[ 5 < 'b' collate NONE ]], {1, "Collation 'NONE' does not exist"}},
+    { 9,  [[ 5 < 'b' collate "none" ]], {0, {1}}},

>> In other words, each field with type of STRING/SCALAR and mb ANY
>> would have real pointer to collation. However, in turn such change
>> would break too many tests, without significant refactoring of SQL
>> query planner (at least I failed to do it in several hours).
>> On the other hand, in this case we can remove COLL_NONE
>> and unify collation interface: each field or key part MUST feature
>> collation. And I mean not only valid ID (it is how it works now),
>> but also valid (i.e. != NULL) pointer to collation struct. So, it seems
>> that we would be able to remove any checks like:
>> if (coll != NULL)
>> 	*use collation*
>> else
>> 	*process without involving collations*
>> Would become:
>> If (type_is_string_like)
>> 	*always use collation*
>> To be honest I was sure that you would notice this pickle…
>> Or current approach is not dat bad?
> 
> Yeah, I understand your aspiration, but I have some remarks
> about the draft patch above and about the concept as it is:
> 
> * you said that you want to get rid of COLL_NONE, but in the draft
>  above you still use it. Even if you had removed COLL_NONE
>  usage from key_def builders, it would have stayed in alter.cc
>  as a check that you can not drop 'none' collation. I think that
>  COLL_NONE as a constant equal to 0 should not be removed from
>  source code until it is not used at all. But it is a minor
>  comment rather about coding style: I prefer code like
>  'id != UNDERSTANDABLE_NAME' to 'id != <strange_constant>'.

And still we use iid != 0 to check whether index primary or not…
But I like your argument.

> * the main flaw in storing a collation for each string is that it
>  will dramatically slow down comparators. The awful problem is in
>  virtual functions calling - it is much slower than regular
>  functions.

Yep, I understand this point and I told the same to Kostja,
but he insisted on this “none” collation. To be honest, I thought
that he wanted to implement second variant, where pointers
to “none” collation != NULL.

> Now we have two levels of avoiding coll->cmp() calling.
>  First in *_compare_create functions. If a key has no collations,
>  we can use highly specified template recursive comparators
>  (struct TupleCompare, struct TupleCompareWithKey) which call
>  memcmp. Second, we are trying to do not call coll->cmp() in other
>  comparators (see mp_compare_str).
> 
>  Yes, we still can follow you, store a collation object in each
>  key_part, tuple_field, field_def, but in tuple_compare.cc we
>  still will need a way how to check that a collation comparator
>  can be inlined into memcmp. This way is check coll->id == COLL_NONE.
>  Now it is like
> 
>      use_fast_cmp = coll == NULL
> 
>  Will be like
> 
>      use_fast_cmp = coll->id == COLL_NONE
> 
>  As a summary: we can store a collation in each *_field/part object
>  to simplify SQL source code, but it does not affect necessity of
>  a special COLL_NONE constant.

Thanks for your comments.

> Also, I see now a new minor problem:
> 
>> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
>> index 64f74f9d3..a9525058b 100644
>> --- a/src/box/lua/upgrade.lua
>> +++ b/src/box/lua/upgrade.lua
>> @@ -396,8 +396,10 @@ local function create_collation_space()
>>     box.space._index:insert{_collation.id, 1, 'name', 'tree', {unique = true}, {{1, 'string'}}}
>>      log.info("create predefined collations")
>> +    box.space._collation:replace{0, "none", ADMIN, "BINARY", "", setmap{}}
>>     box.space._collation:replace{1, "unicode", ADMIN, "ICU", "", setmap{}}
>>     box.space._collation:replace{2, "unicode_ci", ADMIN, "ICU", "", {strength='primary'}}
>> +    box.space._collation:replace{3, "binary", ADMIN, "BINARY", "", setmap{}}
>>      local _priv = box.space[box.schema.PRIV_ID]
>>     _priv:insert{ADMIN, PUBLIC, 'space', _collation.id, box.priv.W}
> 
> Unfortunately, we can not change old update() functions. So the only way to add
> these new collations is to add a new code to upgrade_to_2_1_0() with these two
> new lines.

Ops, indeed. Moved new collations to upgrade function and
updated bootstrap file:

diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index a9525058b..4b0fd0345 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -396,10 +396,8 @@ local function create_collation_space()
     box.space._index:insert{_collation.id, 1, 'name', 'tree', {unique = true}, {{1, 'string'}}}
 
     log.info("create predefined collations")
-    box.space._collation:replace{0, "none", ADMIN, "BINARY", "", setmap{}}
     box.space._collation:replace{1, "unicode", ADMIN, "ICU", "", setmap{}}
     box.space._collation:replace{2, "unicode_ci", ADMIN, "ICU", "", {strength='primary'}}
-    box.space._collation:replace{3, "binary", ADMIN, "BINARY", "", setmap{}}
 
     local _priv = box.space[box.schema.PRIV_ID]
     _priv:insert{ADMIN, PUBLIC, 'space', _collation.id, box.priv.W}
@@ -608,6 +606,9 @@ local function upgrade_to_2_1_0()
     format[2] = {type='any', name='value', is_nullable=true}
     box.space._schema:format(format)
 
+    box.space._collation:replace{0, "none", ADMIN, "BINARY", "", setmap{}}
+    box.space._collation:replace{3, "binary", ADMIN, "BINARY", "", setmap{}}
+
     upgrade_priv_to_2_1_0()
 end

New commit in patch-set:

commit 854792f46d4ec87c3c2218c7645fe398e007c888
Author: Nikita Pettik <korablev@tarantool.org>
Date:   Wed Nov 14 21:07:58 2018 +0300

    sql: don't uppercase name of binary collation
    
    Since now we don't have real binary collation, it was allowed to use its
    name in different cases. For instance: BiNaRY, "bInArY", "BINARY" etc.
    All these names were valid and accounted for binary collation.
    However, we are going to introduce real entry in _collation space to
    represent binary collation. Thus, for now we allow using only standard
    "binary" name.
    
    Needed for #3185

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 17c20480e..191eb0308 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2400,11 +2400,8 @@ index_fill_def(struct Parse *parse, struct index *index,
                if (expr->op == TK_COLLATE) {
                        sql_get_coll_seq(parse, expr->u.zToken, &coll_id);
                        if (coll_id == COLL_NONE &&
-                           strcmp(expr->u.zToken, "binary") != 0) {
-                               diag_set(ClientError, ER_NO_SUCH_COLLATION,
-                                        expr->u.zToken);
+                           strcmp(expr->u.zToken, "binary") != 0)
                                goto tnt_error;
-                       }

diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
index 3cf3a835d..62904fd20 100644
--- a/src/box/sql/callback.c
+++ b/src/box/sql/callback.c
@@ -42,15 +42,16 @@
 struct coll *
 sql_get_coll_seq(Parse *parser, const char *name, uint32_t *coll_id)
 {
-       if (name == NULL || strcasecmp(name, "binary") == 0) {
+       if (name == NULL || strcmp(name, "binary") == 0) {
                *coll_id = COLL_NONE;
                return NULL;
        }
        struct coll_id *p = coll_by_name(name, strlen(name));
        if (p == NULL) {
                *coll_id = COLL_NONE;
-               sqlite3ErrorMsg(parser, "no such collation sequence: %s",
-                               name);
+               diag_set(ClientError, ER_NO_SUCH_COLLATION, name);
+               parser->rc = SQL_TARANTOOL_ERROR;
+               parser->nErr++;
                return NULL;
        } else {
                *coll_id = p->id;
diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua
index eb4f43a90..9483632b2 100755
--- a/test/sql-tap/collation.test.lua
+++ b/test/sql-tap/collation.test.lua
@@ -155,7 +155,7 @@ local data_test_unicode_ci = {
 local data_collations = {
     -- default collation = binary
     {"/*COLLATE DEFAULT*/", data_test_binary_1},
-    {"COLLATE BINARY", data_test_binary_1},
+    {"COLLATE \"binary\"", data_test_binary_1},
     {"COLLATE \"unicode\"", data_test_unicode},
     {"COLLATE \"unicode_ci\"", data_test_unicode_ci},
 }
diff --git a/test/sql-tap/distinct.test.lua b/test/sql-tap/distinct.test.lua
index 26a4aace2..8f6ca8ed9 100755
--- a/test/sql-tap/distinct.test.lua
+++ b/test/sql-tap/distinct.test.lua
@@ -124,9 +124,9 @@ local data = {
     {"13.2", 0, "SELECT DISTINCT a, b, c COLLATE \"unicode_ci\" FROM t4"},
     {"14.1", 0, "SELECT DISTINCT a, d COLLATE \"unicode_ci\" FROM t1"},
     {"14.2", 1, "SELECT DISTINCT a, d COLLATE \"unicode_ci\" FROM t4"},
-    {"15 ", 0, "SELECT DISTINCT a, d COLLATE binary FROM t1"},
-    {"16.1", 0, "SELECT DISTINCT a, b, c COLLATE binary FROM t1"},
-    {"16.2", 1, "SELECT DISTINCT a, b, c COLLATE binary FROM t4"},
+    {"15 ", 0, "SELECT DISTINCT a, d COLLATE \"binary\" FROM t1"},
+    {"16.1", 0, "SELECT DISTINCT a, b, c COLLATE \"binary\" FROM t1"},
+    {"16.2", 1, "SELECT DISTINCT a, b, c COLLATE \"binary\" FROM t4"},
     {"17",  0,   --{ \/* Technically, it would be possible to detect that DISTINCT\n            ** is a no-op in cases like the following. But SQLite does not\n            ** do so. *\/\n
     "SELECT DISTINCT t1.id FROM t1, t2 WHERE t1.id=t2.x" },
     {"18 ", 1, "SELECT DISTINCT c1, c2 FROM t3"},
@@ -173,7 +173,7 @@ data = {
     {"a, b, c FROM t1", {"btree"}, {"A", "B", "C", "a", "b", "c"}},
     {"a, b, c FROM t1 ORDER BY a, b, c", {"btree"}, {"A", "B", "C", "a", "b", "c"}},
     {"b FROM t1 WHERE a = 'a'", {}, {"b"}},
-    {"b FROM t1 ORDER BY +b COLLATE binary", {"btree", "btree"}, {"B", "b"}},
+    {"b FROM t1 ORDER BY +b COLLATE \"binary\"", {"btree", "btree"}, {"B", "b"}},
     {"a FROM t1", {}, {"A", "a"}},
     {"b COLLATE \"unicode_ci\" FROM t1", {}, {"b"}},
     {"b COLLATE \"unicode_ci\" FROM t1 ORDER BY b COLLATE \"unicode_ci\"", {}, {"b"}},
diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua
index 47fb7a809..1e8f096e1 100755
--- a/test/sql-tap/e_select1.test.lua
+++ b/test/sql-tap/e_select1.test.lua
@@ -508,11 +508,11 @@ if (0 > 0)
     test:do_execsql_test(
         "e_select-1.6.0",
         [[
-            CREATE TABLE t5(a  TEXT COLLATE "unicode_ci", b  TEXT COLLATE binary);
+            CREATE TABLE t5(a  TEXT COLLATE "unicode_ci", b  TEXT COLLATE "binary");
             INSERT INTO t5 VALUES('AA', 'cc');
             INSERT INTO t5 VALUES('BB', 'dd');
             INSERT INTO t5 VALUES(NULL, NULL);
-            CREATE TABLE t6(a  TEXT COLLATE binary, b  TEXT COLLATE "unicode_ci");
+            CREATE TABLE t6(a  TEXT COLLATE "c", b  TEXT COLLATE "unicode_ci");
             INSERT INTO t6 VALUES('aa', 'cc');
             INSERT INTO t6 VALUES('bb', 'DD');
             INSERT INTO t6 VALUES(NULL, NULL);
@@ -997,7 +997,7 @@ test:do_execsql_test(
         INSERT INTO b2 VALUES('abc', 3);
         INSERT INTO b2 VALUES('xyz', 4);
 
-        CREATE TABLE b3(id  INT PRIMARY KEY, a  TEXT COLLATE "unicode_ci", b  TEXT COLLATE binary);
+        CREATE TABLE b3(id  INT PRIMARY KEY, a  TEXT COLLATE "unicode_ci", b  TEXT COLLATE "binary");
         INSERT INTO b3 VALUES(1, 'abc', 'abc');
         INSERT INTO b3 VALUES(2, 'aBC', 'aBC');
         INSERT INTO b3 VALUES(3, 'Def', 'Def');
@@ -1289,7 +1289,7 @@ test:do_select_tests(
         {"1", "SELECT DISTINCT b FROM h1", {"one", "I", "i", "four", "IV", "iv"}},
         {"2", "SELECT DISTINCT b COLLATE \"unicode_ci\" FROM h1", {"one", "I", "four", "IV"}},
         {"3", "SELECT DISTINCT x FROM h2", {"One", "Two", "Three", "Four"}},
-        {"4", "SELECT DISTINCT x COLLATE binary FROM h2", {
+        {"4", "SELECT DISTINCT x COLLATE \"binary\" FROM h2", {
             "One", "Two", "Three", "Four", "one", "two", "three", "four"
         }},
     })
@@ -1574,7 +1574,7 @@ test:drop_all_tables()
 test:do_execsql_test(
     "e_select-7.10.0",
     [[
-        CREATE TABLE y1(a  TEXT COLLATE "unicode_ci" PRIMARY KEY, b  TEXT COLLATE binary, c TEXT );
+        CREATE TABLE y1(a  TEXT COLLATE "unicode_ci" PRIMARY KEY, b  TEXT COLLATE "binary", c TEXT );
         INSERT INTO y1 VALUES('Abc', 'abc', 'aBC');
     ]], {
         -- <e_select-7.10.0>
@@ -1588,12 +1588,12 @@ test:do_select_tests(
         {"1", "SELECT 'abc'                UNION SELECT 'ABC'", {"ABC",  "abc"}},
         {"2", "SELECT 'abc' COLLATE \"unicode_ci\" UNION SELECT 'ABC'", {"ABC" }},
         {"3", "SELECT 'abc'                UNION SELECT 'ABC' COLLATE \"unicode_ci\"", {"ABC" }},
-        {"4", "SELECT 'abc' COLLATE binary UNION SELECT 'ABC' COLLATE \"unicode_ci\"", {"ABC",  "abc"}},
-        {"5", "SELECT 'abc' COLLATE \"unicode_ci\" UNION SELECT 'ABC' COLLATE binary", {"ABC" }},
+        {"4", "SELECT 'abc' COLLATE \"binary\" UNION SELECT 'ABC' COLLATE \"unicode_ci\"", {"ABC",  "abc"}},
+        {"5", "SELECT 'abc' COLLATE \"unicode_ci\" UNION SELECT 'ABC' COLLATE \"binary\"", {"ABC" }},
         {"6", "SELECT a FROM y1 UNION SELECT b FROM y1", {"abc" }},
         {"7", "SELECT b FROM y1 UNION SELECT a FROM y1", {"Abc",  "abc"}},
         {"8", "SELECT a FROM y1 UNION SELECT c FROM y1", {"aBC" }},
-        {"9", "SELECT a FROM y1 UNION SELECT c COLLATE binary FROM y1", {"aBC" }},
+        {"9", "SELECT a FROM y1 UNION SELECT c COLLATE \"binary\" FROM y1", {"aBC" }},
     })
 
 -- EVIDENCE-OF: R-32706-07403 No affinity transformations are applied to
@@ -1932,7 +1932,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "e_select-8.9.1",
     [[
-        SELECT x FROM d4 ORDER BY 1 COLLATE binary
+        SELECT x FROM d4 ORDER BY 1 COLLATE "binary"
     ]], {
         -- <e_select-8.9.1>
         "DEF", "JKL", "abc", "ghi"
@@ -1942,7 +1942,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "e_select-8.9.2",
     [[
-        SELECT x COLLATE binary FROM d4 ORDER BY 1 COLLATE "unicode_ci"
+        SELECT x COLLATE "binary" FROM d4 ORDER BY 1 COLLATE "unicode_ci"
     ]], {
         -- <e_select-8.9.2>
         "abc", "DEF", "ghi", "JKL"
@@ -1963,7 +1963,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "e_select-8.10.1",
     [[
-        SELECT x COLLATE binary FROM d4 ORDER BY 1
+        SELECT x COLLATE "binary" FROM d4 ORDER BY 1
     ]], {
         -- <e_select-8.10.1>
         "DEF", "JKL", "abc", "ghi"
@@ -1973,7 +1973,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "e_select-8.10.2",
     [[
-        SELECT x COLLATE binary FROM d4 ORDER BY x
+        SELECT x COLLATE "binary" FROM d4 ORDER BY x
     ]], {
         -- <e_select-8.10.2>
         "abc", "DEF", "ghi", "JKL"
@@ -1983,7 +1983,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "e_select-8.10.3",
     [[
-        SELECT x COLLATE binary AS x FROM d4 ORDER BY x
+        SELECT x COLLATE "binary" AS x FROM d4 ORDER BY x
     ]], {
         -- <e_select-8.10.3>
         "DEF", "JKL", "abc", "ghi"
diff --git a/test/sql-tap/identifier_case.test.lua b/test/sql-tap/identifier_case.test.lua
index 096130a52..4878ced65 100755
--- a/test/sql-tap/identifier_case.test.lua
+++ b/test/sql-tap/identifier_case.test.lua
@@ -201,10 +201,10 @@ end
 
 -- Check that collaiton names work as identifiers
 data = {
-    { 1,  [[ binary ]], {0}},
-    { 2,  [[ BINARY ]], {0}},
+    { 1,  [[ binary ]], {1, "Collation 'BINARY' does not exist"}},
+    { 2,  [[ BINARY ]], {1, "Collation 'BINARY' does not exist"}},
     { 3,  [["binary"]], {0}},
-    { 4,  [["bInaRy"]], {0}},
+    { 4,  [["bInaRy"]], {1, "Collation 'bInaRy' does not exist"}},
     { 5,  [["unicode"]], {0}},
     { 6,  [[ unicode ]], {1,"Collation 'UNICODE' does not exist"}},
     { 7,  [["UNICODE"]], {1,"Collation 'UNICODE' does not exist"}}
@@ -234,12 +234,12 @@ test:do_catchsql_test(
     {0})
 
 data = {
-    { 1,  [[ 'a' < 'b' collate binary ]], {0, {1}}},
+    { 1,  [[ 'a' < 'b' collate binary ]], {1, "Collation 'BINARY' does not exist"}},
     { 2,  [[ 'a' < 'b' collate "binary" ]], {0, {1}}},
     { 3,  [[ 'a' < 'b' collate 'binary' ]], {1, [[near "'binary'": syntax error]]}},
     { 4,  [[ 'a' < 'b' collate "unicode" ]], {0, {1}}},
     { 5,  [[ 5 < 'b' collate "unicode" ]], {0, {1}}},
-    { 6,  [[ 5 < 'b' collate unicode ]], {1,"no such collation sequence: UNICODE"}},
+    { 6,  [[ 5 < 'b' collate unicode ]], {1,"Collation 'UNICODE' does not exist"}},
     { 7,  [[ 5 < 'b' collate "unicode_ci" ]], {0, {1}}},
 }
 
diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua
index 83139a3e4..e6f0d415d 100755
--- a/test/sql-tap/in3.test.lua
+++ b/test/sql-tap/in3.test.lua
@@ -206,7 +206,7 @@ test:do_test(
 test:do_test(
     "in3-1.15",
     function()
-        return exec_neph(" SELECT a FROM t1 WHERE a COLLATE binary IN (SELECT a FROM t1) ")
+        return exec_neph(" SELECT a FROM t1 WHERE a COLLATE \"binary\" IN (SELECT a FROM t1) ")
     end, {
         -- <in3-1.15>
         0, 1, 3, 5
diff --git a/test/sql-tap/in5.test.lua b/test/sql-tap/in5.test.lua
index 4e2cdcd24..ba21170ee 100755
--- a/test/sql-tap/in5.test.lua
+++ b/test/sql-tap/in5.test.lua
@@ -277,7 +277,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "6.1.2",
     [[
-        SELECT count(*) FROM t1 WHERE a COLLATE BINARY IN (SELECT DISTINCT a FROM t1)
+        SELECT count(*) FROM t1 WHERE a COLLATE "binary" IN (SELECT DISTINCT a FROM t1)
     ]], {
         -- <6.1.2>
         1
diff --git a/test/sql-tap/index3.test.lua b/test/sql-tap/index3.test.lua
index 4f950be09..222b895aa 100755
--- a/test/sql-tap/index3.test.lua
+++ b/test/sql-tap/index3.test.lua
@@ -59,7 +59,7 @@ test:do_execsql_test(
         CREATE TABLE t1(a INT , b TEXT , c INT , d INT , e INT ,
                         PRIMARY KEY(a), UNIQUE(b COLLATE "unicode_ci" DESC));
         CREATE INDEX t1c ON t1(c);
-        CREATE INDEX t1d ON t1(d COLLATE binary ASC);
+        CREATE INDEX t1d ON t1(d COLLATE "binary" ASC);
         WITH RECURSIVE c(x) AS (VALUES(1) UNION SELECT x+1 FROM c WHERE x<30)
           INSERT INTO t1(a,b,c,d,e) 
             SELECT x, printf('ab%03xxy',x), x, x, x FROM c;
diff --git a/test/sql-tap/minmax3.test.lua b/test/sql-tap/minmax3.test.lua
index 1ddf39ff5..348275ffa 100755
--- a/test/sql-tap/minmax3.test.lua
+++ b/test/sql-tap/minmax3.test.lua
@@ -502,7 +502,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "minmax3-4.4",
     [[
-        SELECT max(x COLLATE binary), max(x COLLATE "unicode_ci") FROM t4;
+        SELECT max(x COLLATE "binary"), max(x COLLATE "unicode_ci") FROM t4;
     ]], {
         -- <minmax3-4.4>
         "abc", "BCD"
@@ -564,7 +564,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "minmax3-4.13",
     [[
-        SELECT min(x COLLATE binary), min(x COLLATE "unicode_ci") FROM t4;
+        SELECT min(x COLLATE "binary"), min(x COLLATE "unicode_ci") FROM t4;
     ]], {
         -- <minmax3-4.13>
         "BCD", "abc"
diff --git a/test/sql-tap/resolver01.test.lua b/test/sql-tap/resolver01.test.lua
index d08f95bf6..7be2184e5 100755
--- a/test/sql-tap/resolver01.test.lua
+++ b/test/sql-tap/resolver01.test.lua
@@ -236,7 +236,7 @@ test:do_test(
         ]])
         local r = {}
         table.insert(r, test:execsql("SELECT '1', substr(m,2) AS m FROM t4 ORDER BY m;"))
-        table.insert(r, test:execsql("SELECT '2', substr(m,2) AS m FROM t4 ORDER BY m COLLATE binary;"))
+        table.insert(r, test:execsql("SELECT '2', substr(m,2) AS m FROM t4 ORDER BY m COLLATE \"binary\";"))
         table.insert(r, test:execsql("SELECT '3', substr(m,2) AS m FROM t4 ORDER BY lower(m);"))
         return r
     end, {
diff --git a/test/sql-tap/selectE.test.lua b/test/sql-tap/selectE.test.lua
index 11b84711e..b379f5aa8 100755
--- a/test/sql-tap/selectE.test.lua
+++ b/test/sql-tap/selectE.test.lua
@@ -84,7 +84,7 @@ test:do_test(
     function()
         return test:execsql [[
             SELECT a FROM t2 EXCEPT SELECT a FROM t3
-             ORDER BY a COLLATE binary;
+             ORDER BY a COLLATE "binary";
         ]]
     end, {
         -- <selectE-1.2>
@@ -127,7 +127,7 @@ test:do_test(
     function()
         return test:execsql [[
             SELECT a COLLATE "unicode_ci" FROM t2 EXCEPT SELECT a FROM t3
-             ORDER BY 1 COLLATE binary
+             ORDER BY 1 COLLATE "binary"
         ]]
     end, {
         -- <selectE-2.2>
diff --git a/test/sql-tap/tkt2391.test.lua b/test/sql-tap/tkt2391.test.lua
index 7fa5e1634..b48998c53 100755
--- a/test/sql-tap/tkt2391.test.lua
+++ b/test/sql-tap/tkt2391.test.lua
@@ -20,7 +20,7 @@ test:plan(4)
 test:do_execsql_test(
     "tkt2391.1",
     [[
-        CREATE TABLE folders(folderid INT , parentid INT , foldername TEXT COLLATE binary primary key);
+        CREATE TABLE folders(folderid INT , parentid INT , foldername TEXT COLLATE "binary" primary key);
         INSERT INTO folders VALUES(1, 3, 'FolderA');
         INSERT INTO folders VALUES(1, 3, 'folderB');
         INSERT INTO folders VALUES(4, 0, 'FolderC');
diff --git a/test/sql-tap/tkt3493.test.lua b/test/sql-tap/tkt3493.test.lua
index 26ca2271b..7e093b217 100755
--- a/test/sql-tap/tkt3493.test.lua
+++ b/test/sql-tap/tkt3493.test.lua
@@ -246,7 +246,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "tkt3493-3.1",
     [[
-        CREATE TABLE t2(a  TEXT COLLATE "unicode_ci" PRIMARY KEY, b  TEXT COLLATE BINARY);
+        CREATE TABLE t2(a  TEXT COLLATE "unicode_ci" PRIMARY KEY, b  TEXT COLLATE "binary");
         INSERT INTO t2 VALUES('aBc', 'DeF');
     ]], {
         -- <tkt3493-3.1>
@@ -291,7 +291,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "tkt3493-3.3.2",
     [[
-        SELECT a>b COLLATE BINARY FROM t2 GROUP BY a, b
+        SELECT a>b COLLATE "binary" FROM t2 GROUP BY a, b
     ]], {
         -- <tkt3493-3.3.2>
         1
diff --git a/test/sql-tap/with1.test.lua b/test/sql-tap/with1.test.lua
index faa99811c..8200d25f8 100755
--- a/test/sql-tap/with1.test.lua
+++ b/test/sql-tap/with1.test.lua
@@ -1056,7 +1056,7 @@ test:do_catchsql_test(13.3, [[
 -- 2015-04-12
 --
 test:do_execsql_test(14.1, [[
-  WITH x AS (SELECT * FROM t) SELECT 0 EXCEPT SELECT 0 ORDER BY 1 COLLATE binary;
+  WITH x AS (SELECT * FROM t) SELECT 0 EXCEPT SELECT 0 ORDER BY 1 COLLATE "binary";
 ]], {
   -- <14.1>