Tarantool development patches archive
 help / color / mirror / Atom feed
From: Nikita Pettik <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: v.shpilevoy@tarantool.org, Nikita Pettik <korablev@tarantool.org>
Subject: [tarantool-patches] [PATCH 2/3] Add surrogate ID for BINARY collation
Date: Thu, 25 Oct 2018 14:00:08 +0300	[thread overview]
Message-ID: <80794eb0182261e1887adc60c170c550de91fabc.1540460716.git.korablev@tarantool.org> (raw)
In-Reply-To: <cover.1540460716.git.korablev@tarantool.org>
In-Reply-To: <cover.1540460716.git.korablev@tarantool.org>

BINARY collation is not really collation - no one object represents it.
It is set by default, unless otherwise stated. However, according to
ANSI SQL there is difference between "no collation" and "explicitly
specified BINARY collation". So, alongside with COLL_NONE id indicating
the absence of collation, lets introduce COLL_BINARY id to outline the
fact that BINARY collation is set and should be forced during string
comparison.

Part of #3185
---
 src/box/key_def.c           |  2 +-
 src/box/key_def.h           | 17 +++++++++++++++++
 src/box/lua/space.cc        |  2 +-
 src/box/sql/callback.c      |  6 +++++-
 src/box/tuple_format.c      |  2 +-
 test/sql/collation.result   | 16 ++++++++++++++++
 test/sql/collation.test.lua |  8 ++++++++
 7 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/src/box/key_def.c b/src/box/key_def.c
index 3a560bb06..dd47e5d75 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -174,7 +174,7 @@ key_def_new(const struct key_part_def *parts, uint32_t part_count)
 	for (uint32_t i = 0; i < part_count; i++) {
 		const struct key_part_def *part = &parts[i];
 		struct coll *coll = NULL;
-		if (part->coll_id != COLL_NONE) {
+		if (! coll_is_missing(part->coll_id)) {
 			struct coll_id *coll_id = coll_by_id(part->coll_id);
 			if (coll_id == NULL) {
 				diag_set(ClientError, ER_WRONG_INDEX_OPTIONS,
diff --git a/src/box/key_def.h b/src/box/key_def.h
index 20e79f9fe..ecdc199d9 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -78,6 +78,23 @@ extern const struct key_part_def key_part_def_default;
  */
 #define COLL_NONE UINT32_MAX
 
+/**
+ * In SQL explicitly specified binary collation and absence of
+ * any collation are different in behaviour: according to ANSI
+ * it is prohibited to compare strings with different explicitly
+ * indicated collations. However, if one of collation is default,
+ * (i.e. absent) the second one will be forced.
+ * So, lets introduce another id to indicate explicitly specified
+ * binary collation.
+ */
+#define COLL_BINARY (UINT32_MAX - 1)
+
+static inline bool
+coll_is_missing(uint32_t coll_id)
+{
+	return coll_id == COLL_NONE || coll_id == COLL_BINARY;
+}
+
 /** Descriptor of a single part in a multipart key. */
 struct key_part {
 	/** Tuple field index for this part */
diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index c75ba4782..0207639a1 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -299,7 +299,7 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i)
 			lua_pushboolean(L, key_part_is_nullable(part));
 			lua_setfield(L, -2, "is_nullable");
 
-			if (part->coll_id != COLL_NONE) {
+			if (! coll_is_missing(part->coll_id)) {
 				struct coll_id *coll_id =
 					coll_by_id(part->coll_id);
 				assert(coll_id != NULL);
diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
index 3cf3a835d..d4789257f 100644
--- a/src/box/sql/callback.c
+++ b/src/box/sql/callback.c
@@ -42,10 +42,14 @@
 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) {
 		*coll_id = COLL_NONE;
 		return NULL;
 	}
+	if (strcasecmp(name, "binary") == 0) {
+		*coll_id = COLL_BINARY;
+		return NULL;
+	}
 	struct coll_id *p = coll_by_name(name, strlen(name));
 	if (p == NULL) {
 		*coll_id = COLL_NONE;
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 1b36a53d6..2f28f18df 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -67,7 +67,7 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
 		format->fields[i].nullable_action = fields[i].nullable_action;
 		struct coll *coll = NULL;
 		uint32_t cid = fields[i].coll_id;
-		if (cid != COLL_NONE) {
+		if (! coll_is_missing(cid)) {
 			struct coll_id *coll_id = coll_by_id(cid);
 			if (coll_id == NULL) {
 				diag_set(ClientError,ER_WRONG_COLLATION_OPTIONS,
diff --git a/test/sql/collation.result b/test/sql/collation.result
index 79ba9abc0..f6254f866 100644
--- a/test/sql/collation.result
+++ b/test/sql/collation.result
@@ -107,6 +107,22 @@ cn:execute('select 1 limit ? collate not_exist', {1})
 cn:close()
 ---
 ...
+-- Explicitly set BINARY collation has ID.
+--
+box.sql.execute("CREATE TABLE t (id INT PRIMARY KEY, a TEXT, b TEXT COLLATE BINARY);")
+---
+...
+box.space.T:format()[2]['collation']
+---
+- null
+...
+box.space.T:format()[3]['collation']
+---
+- 4294967294
+...
+box.sql.execute("DROP TABLE t;")
+---
+...
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 ---
 ...
diff --git a/test/sql/collation.test.lua b/test/sql/collation.test.lua
index 935dea824..f9d653717 100644
--- a/test/sql/collation.test.lua
+++ b/test/sql/collation.test.lua
@@ -42,4 +42,12 @@ cn = remote.connect(box.cfg.listen)
 cn:execute('select 1 limit ? collate not_exist', {1})
 
 cn:close()
+
+-- Explicitly set BINARY collation has ID.
+--
+box.sql.execute("CREATE TABLE t (id INT PRIMARY KEY, a TEXT, b TEXT COLLATE BINARY);")
+box.space.T:format()[2]['collation']
+box.space.T:format()[3]['collation']
+box.sql.execute("DROP TABLE t;")
+
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
-- 
2.15.1

  parent reply	other threads:[~2018-10-25 11:00 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-25 11:00 [tarantool-patches] [PATCH 0/3] Change collation compatibility rules according to ANSI SQL Nikita Pettik
2018-10-25 11:00 ` [tarantool-patches] [PATCH 1/3] sql: do not add explicit <COLLATE "BINARY"> clause Nikita Pettik
2018-10-25 11:00 ` Nikita Pettik [this message]
2018-10-31 12:34   ` [tarantool-patches] Re: [PATCH 2/3] Add surrogate ID for BINARY collation Vladislav Shpilevoy
2018-10-31 15:47     ` n.pettik
2018-11-01 11:37       ` Konstantin Osipov
2018-11-01 12:22         ` Vladislav Shpilevoy
2018-11-01 12:58           ` Konstantin Osipov
2018-11-01 13:08             ` n.pettik
2018-11-01 15:39               ` Konstantin Osipov
     [not found]                 ` <95CB17D5-E3ED-4B05-A289-983E2FD0DE37@gmail.com>
2018-11-01 17:45                   ` n.pettik
2018-11-01 20:00                   ` Konstantin Osipov
2018-11-01 20:06                     ` Konstantin Osipov
2018-11-01 20:20                     ` n.pettik
2018-10-25 11:00 ` [tarantool-patches] [PATCH 3/3] sql: change collation compatibility rules Nikita Pettik
2018-10-31 12:34   ` [tarantool-patches] " Vladislav Shpilevoy
2018-11-12 23:46     ` n.pettik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=80794eb0182261e1887adc60c170c550de91fabc.1540460716.git.korablev@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] [PATCH 2/3] Add surrogate ID for BINARY collation' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox