Tarantool development patches archive
 help / color / mirror / Atom feed
From: imeevma@tarantool.org
To: tarantool-patches@freelists.org, korablev@tarantool.org
Subject: [tarantool-patches] [PATCH v3 4/6] sql: fix "PRAGMA case_sensitive_like" result
Date: Wed, 26 Dec 2018 21:18:17 +0300	[thread overview]
Message-ID: <2589721198edcf039cfdc98a18d9e0670e6e67af.1545844480.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1545844480.git.imeevma@gmail.com>

Hi! Thank you for review! My answers, diff between versions and
new version below.


On 12/24/18 5:01 PM, n.pettik wrote:
> Nit: since pragma is called ‘case_sensitive_like’, I would revert variable
> meaning:
This part was removed in new version.

> Also, the same question here: why you didn’t make it be of
> type ‘flag’? The only difference in additional call of sqlite3RegisterLikeFunctions.
Done. Moved to 'flag'.

> Lets avoid using ’sqlite’ prefixes for added code.
Fixed.


Diff between version:

commit 4662242039ad7c02b5401ecc5e21c3aa4707039f
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Tue Dec 25 19:17:11 2018 +0300

    Temporary: review fixes 2

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 2651cad..67defed 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -488,6 +488,15 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 					sqlite3ParserTrace(0, 0);
 			}
 #endif
+			/*
+			 * Reinstall the LIKE and functions. The
+			 * variant of LIKE * used will be case
+			 * sensitive or not depending on the RHS.
+			 */
+			if (mask == LIKE_CASE_SENS_FLAG) {
+				sqlite3RegisterLikeFunctions(db,
+							     !is_value_true);
+			}
 		}
 		break;
 	}
@@ -568,27 +577,6 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 		break;
 	}
 
-		/*
-		 * Reinstall the LIKE and functions. The variant
-		 * of LIKE * used will be case sensitive or not
-		 * depending on the RHS.
-		 */
-	case PragTyp_CASE_SENSITIVE_LIKE:{
-		int mask = pPragma->iArg;
-		if (zRight == NULL) {
-			returnSingleInt(v,
-					(user_session->sql_flags & mask) != 0);
-		} else {
-			int is_like_ci = !(sqlite3GetBoolean(zRight, 0));
-			if (!is_like_ci)
-				user_session->sql_flags |= mask;
-			else
-				user_session->sql_flags &= ~mask;
-			sqlite3RegisterLikeFunctions(db, is_like_ci);
-		}
-		break;
-	}
-
 	case PragTyp_DEFAULT_ENGINE: {
 		if (zRight == NULL) {
 			const char *engine_name =
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index fed4b33..07700e7 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -6,7 +6,6 @@
 
 /* The various pragma types */
 #define PragTyp_BUSY_TIMEOUT                   1
-#define PragTyp_CASE_SENSITIVE_LIKE            2
 #define PragTyp_COLLATION_LIST                 3
 #define PragTyp_FLAG                           5
 #define PragTyp_FOREIGN_KEY_LIST               9
@@ -92,10 +91,10 @@ static const PragmaName aPragmaName[] = {
 	 /* ColNames:  */ 31, 1,
 	 /* iArg:      */ 0},
 	{ /* zName:     */ "case_sensitive_like",
-	 /* ePragTyp:  */ PragTyp_CASE_SENSITIVE_LIKE,
+	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
 	 /* ColNames:  */ 0, 0,
-	 /* iArg:      */ SQLITE_LikeIsCaseSens},
+	 /* iArg:      */ LIKE_CASE_SENS_FLAG},
 	{ /* zName:     */ "collation_list",
 	 /* ePragTyp:  */ PragTyp_COLLATION_LIST,
 	 /* ePragFlg:  */ PragFlg_Result0,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 334b12e..9c53fe9 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1565,9 +1565,9 @@ struct sqlite3 {
 #define SQLITE_VdbeTrace      0x00000001	/* True to trace VDBE execution */
 /* Debug print info about SQL query as it parsed */
 #define PARSER_TRACE_FLAG     0x00000002
-/* True if LIKE is case sensitive. */
-#define SQLITE_LikeIsCaseSens 0x00000008
 #define SQLITE_FullColNames   0x00000004	/* Show full column names on SELECT */
+/* True if LIKE is case sensitive. */
+#define LIKE_CASE_SENS_FLAG   0x00000008
 #define SQLITE_ShortColNames  0x00000040	/* Show short columns names */
 #define SQLITE_CountRows      0x00000080	/* Count rows changed by INSERT, */
 					  /*   DELETE, or UPDATE and return */
diff --git a/test/sql/misc.result b/test/sql/misc.result
index f1031f7..66f5a7b 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -47,13 +47,16 @@ box.sql.execute('\n\n\n\t\t\t   ')
 result = box.sql.execute('PRAGMA case_sensitive_like')
 ---
 ...
--- Should be TRUE.
-result[1][1] == 0 or result[1][1] == 1
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = 1')
 ---
-- true
 ...
--- Check that "PRAGMA case_sensitive_like" returns nothing if
--- called with parameter.
+-- Should be 1.
+box.sql.execute('PRAGMA case_sensitive_like')
+---
+- - [1]
+...
+-- Should be nothing.
 box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
 ---
 ...
diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
index 5952035..cc31a5d 100644
--- a/test/sql/misc.test.lua
+++ b/test/sql/misc.test.lua
@@ -18,9 +18,9 @@ box.sql.execute('\n\n\n\t\t\t   ')
 -- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
 -- called without parameter.
 result = box.sql.execute('PRAGMA case_sensitive_like')
--- Should be TRUE.
-result[1][1] == 0 or result[1][1] == 1
-
--- Check that "PRAGMA case_sensitive_like" returns nothing if
--- called with parameter.
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = 1')
+-- Should be 1.
+box.sql.execute('PRAGMA case_sensitive_like')
+-- Should be nothing.
 box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])


New version:

commit 2589721198edcf039cfdc98a18d9e0670e6e67af
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Thu Dec 13 15:18:54 2018 +0300

    sql: fix "PRAGMA case_sensitive_like" result
    
    Currently PRAGMA case_sensitive_like returns nothing. This seems
    wrong, since other similar pragmas return their status. Fixed in
    the current patch.

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 9b68c13..67defed 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -488,6 +488,15 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 					sqlite3ParserTrace(0, 0);
 			}
 #endif
+			/*
+			 * Reinstall the LIKE and functions. The
+			 * variant of LIKE * used will be case
+			 * sensitive or not depending on the RHS.
+			 */
+			if (mask == LIKE_CASE_SENS_FLAG) {
+				sqlite3RegisterLikeFunctions(db,
+							     !is_value_true);
+			}
 		}
 		break;
 	}
@@ -568,20 +577,6 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 		break;
 	}
 
-		/*
-		 * Reinstall the LIKE and functions. The variant
-		 * of LIKE * used will be case sensitive or not
-		 * depending on the RHS.
-		 */
-	case PragTyp_CASE_SENSITIVE_LIKE:{
-			if (zRight) {
-				int is_like_ci =
-					!(sqlite3GetBoolean(zRight, 0));
-				sqlite3RegisterLikeFunctions(db, is_like_ci);
-			}
-			break;
-		}
-
 	case PragTyp_DEFAULT_ENGINE: {
 		if (zRight == NULL) {
 			const char *engine_name =
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index ae8e030..07700e7 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -6,7 +6,6 @@
 
 /* The various pragma types */
 #define PragTyp_BUSY_TIMEOUT                   1
-#define PragTyp_CASE_SENSITIVE_LIKE            2
 #define PragTyp_COLLATION_LIST                 3
 #define PragTyp_FLAG                           5
 #define PragTyp_FOREIGN_KEY_LIST               9
@@ -92,10 +91,10 @@ static const PragmaName aPragmaName[] = {
 	 /* ColNames:  */ 31, 1,
 	 /* iArg:      */ 0},
 	{ /* zName:     */ "case_sensitive_like",
-	 /* ePragTyp:  */ PragTyp_CASE_SENSITIVE_LIKE,
-	 /* ePragFlg:  */ PragFlg_NoColumns,
+	 /* ePragTyp:  */ PragTyp_FLAG,
+	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
 	 /* ColNames:  */ 0, 0,
-	 /* iArg:      */ 0},
+	 /* iArg:      */ LIKE_CASE_SENS_FLAG},
 	{ /* zName:     */ "collation_list",
 	 /* ePragTyp:  */ PragTyp_COLLATION_LIST,
 	 /* ePragFlg:  */ PragFlg_Result0,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index a484486..9c53fe9 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1566,6 +1566,8 @@ struct sqlite3 {
 /* Debug print info about SQL query as it parsed */
 #define PARSER_TRACE_FLAG     0x00000002
 #define SQLITE_FullColNames   0x00000004	/* Show full column names on SELECT */
+/* True if LIKE is case sensitive. */
+#define LIKE_CASE_SENS_FLAG   0x00000008
 #define SQLITE_ShortColNames  0x00000040	/* Show short columns names */
 #define SQLITE_CountRows      0x00000080	/* Count rows changed by INSERT, */
 					  /*   DELETE, or UPDATE and return */
diff --git a/test/sql/misc.result b/test/sql/misc.result
index ef104c1..66f5a7b 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -40,3 +40,23 @@ box.sql.execute('\n\n\n\t\t\t   ')
 ---
 - error: 'syntax error: empty request'
 ...
+--
+-- gh-3832: Some statements do not return column type
+-- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
+-- called without parameter.
+result = box.sql.execute('PRAGMA case_sensitive_like')
+---
+...
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = 1')
+---
+...
+-- Should be 1.
+box.sql.execute('PRAGMA case_sensitive_like')
+---
+- - [1]
+...
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
+---
+...
diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua
index 994e64f..cc31a5d 100644
--- a/test/sql/misc.test.lua
+++ b/test/sql/misc.test.lua
@@ -11,3 +11,16 @@ box.sql.execute(';')
 box.sql.execute('')
 box.sql.execute('     ;')
 box.sql.execute('\n\n\n\t\t\t   ')
+
+--
+-- gh-3832: Some statements do not return column type
+
+-- Check that "PRAGMA case_sensitive_like" returns 0 or 1 if
+-- called without parameter.
+result = box.sql.execute('PRAGMA case_sensitive_like')
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = 1')
+-- Should be 1.
+box.sql.execute('PRAGMA case_sensitive_like')
+-- Should be nothing.
+box.sql.execute('PRAGMA case_sensitive_like = '.. result[1][1])
-- 
2.7.4

  parent reply	other threads:[~2018-12-26 18:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-26 18:17 [tarantool-patches] [PATCH v3 0/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2018-12-26 18:17 ` [tarantool-patches] [PATCH v3 1/6] sql: remove unused macros from pragma.c and pragma.h imeevma
2019-01-16 15:34   ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 2/6] sql: fix "PRAGMA parser_trace" result imeevma
2019-01-16 15:35   ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 3/6] sql: Show currently set sql_default_engine imeevma
2018-12-26 18:18 ` imeevma [this message]
2019-01-16 15:35   ` [tarantool-patches] Re: [PATCH v3 4/6] sql: fix "PRAGMA case_sensitive_like" result n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 5/6] sql: 'PRAGMA' result in the appropriate format imeevma
2019-01-16 15:35   ` [tarantool-patches] " n.pettik
2018-12-26 18:18 ` [tarantool-patches] [PATCH v3 6/6] sql: set column types for EXPLAIN and PRAGMA imeevma
2019-01-16 15:35   ` [tarantool-patches] " 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=2589721198edcf039cfdc98a18d9e0670e6e67af.1545844480.git.imeevma@gmail.com \
    --to=imeevma@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH v3 4/6] sql: fix "PRAGMA case_sensitive_like" result' \
    /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