Tarantool development patches archive
 help / color / mirror / Atom feed
From: imeevma@tarantool.org
To: korablev@tarantool.org, tsafin@tarantool.org,
	tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH v6 17/22] sql: remove unused DOUBLE to INTEGER conversion
Date: Thu, 16 Jul 2020 17:47:10 +0300	[thread overview]
Message-ID: <a25ad9c5dadec9a13cc35b10fcdcf16476dbc202.1594909974.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1594909974.git.imeevma@gmail.com>

This patch removes the unused DOUBLE to INTEGER conversion from OP_Seek*
opcodes. This transformation is not used due to changes in the ApplyType
opcode. The next few patches will introduce new rules for converting
numbers (not just DOUBLE to INTEGER), and the implicit conversion within
the ApplyType opcode will be disabled for this case.

Part of #4230
---
 src/box/sql/vdbe.c      | 106 ++--------------------------------------
 src/box/sql/wherecode.c |  27 ----------
 2 files changed, 5 insertions(+), 128 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 97e09cac4..d853a0edb 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -3366,7 +3366,7 @@ case OP_Close: {
 	break;
 }
 
-/* Opcode: SeekGE P1 P2 P3 P4 P5
+/* Opcode: SeekGE P1 P2 P3 P4 *
  * Synopsis: key=r[P3@P4]
  *
  * If cursor P1 refers to an SQL table (B-Tree that uses integer keys),
@@ -3389,14 +3389,9 @@ case OP_Close: {
  * from the beginning toward the end.  In other words, the cursor is
  * configured to use Next, not Prev.
  *
- * If P5 is not zero, than it is offset of integer fields in input
- * vector. Force corresponding value to be INTEGER, in case it
- * is floating point value. Alongside with that, type of
- * iterator may be changed: a > 1.5 -> a >= 2.
- *
  * See also: Found, NotFound, SeekLt, SeekGt, SeekLe
  */
-/* Opcode: SeekGT P1 P2 P3 P4 P5
+/* Opcode: SeekGT P1 P2 P3 P4 *
  * Synopsis: key=r[P3@P4]
  *
  * If cursor P1 refers to an SQL table (B-Tree that uses integer keys),
@@ -3412,12 +3407,9 @@ case OP_Close: {
  * from the beginning toward the end.  In other words, the cursor is
  * configured to use Next, not Prev.
  *
- * If P5 is not zero, than it is offset of integer fields in input
- * vector. Force corresponding value to be INTEGER.
- *
- * P5 has the same meaning as for SeekGE.
+ * See also: Found, NotFound, SeekLt, SeekGe, SeekLe
  */
-/* Opcode: SeekLT P1 P2 P3 P4 P5
+/* Opcode: SeekLT P1 P2 P3 P4 *
  * Synopsis: key=r[P3@P4]
  *
  * If cursor P1 refers to an SQL table (B-Tree that uses integer keys),
@@ -3433,11 +3425,9 @@ case OP_Close: {
  * from the end toward the beginning.  In other words, the cursor is
  * configured to use Prev, not Next.
  *
- * P5 has the same meaning as for SeekGE.
- *
  * See also: Found, NotFound, SeekGt, SeekGe, SeekLe
  */
-/* Opcode: SeekLE P1 P2 P3 P4 P5
+/* Opcode: SeekLE P1 P2 P3 P4 *
  * Synopsis: key=r[P3@P4]
  *
  * If cursor P1 refers to an SQL table (B-Tree that uses integer keys),
@@ -3460,8 +3450,6 @@ case OP_Close: {
  * The IdxGE opcode will be skipped if this opcode succeeds, but the
  * IdxGE opcode will be used on subsequent loop iterations.
  *
- * P5 has the same meaning as for SeekGE.
- *
  * See also: Found, NotFound, SeekGt, SeekGe, SeekLt
  */
 case OP_SeekLT:         /* jump, in3 */
@@ -3473,7 +3461,6 @@ case OP_SeekGT: {       /* jump, in3 */
 	VdbeCursor *pC;    /* The cursor to seek */
 	UnpackedRecord r;  /* The key to seek for */
 	int nField;        /* Number of columns or fields in the key */
-	i64 iKey;          /* The id we are to seek to */
 	int eqOnly;        /* Only interested in == results */
 
 	assert(pOp->p1>=0 && pOp->p1<p->nCursor);
@@ -3491,86 +3478,6 @@ case OP_SeekGT: {       /* jump, in3 */
 #ifdef SQL_DEBUG
 	pC->seekOp = pOp->opcode;
 #endif
-	iKey = 0;
-	/*
-	 * In case floating value is intended to be passed to
-	 * iterator over integer field, we must truncate it to
-	 * integer value and change type of iterator:
-	 * a > 1.5 -> a >= 2
-	 */
-	int int_field = pOp->p5;
-	bool is_neg = false;
-
-	if (int_field > 0) {
-		/* The input value in P3 might be of any type: integer, real, string,
-		 * blob, or NULL.  But it needs to be an integer before we can do
-		 * the seek, so convert it.
-		 */
-		pIn3 = &aMem[int_field];
-		if ((pIn3->flags & MEM_Null) != 0)
-			goto skip_truncate;
-		if ((pIn3->flags & MEM_Str) != 0)
-			mem_apply_numeric_type(pIn3);
-		int64_t i;
-		if ((pIn3->flags & MEM_Int) == MEM_Int) {
-			i = pIn3->u.i;
-			is_neg = true;
-		} else if ((pIn3->flags & MEM_UInt) == MEM_UInt) {
-			i = pIn3->u.u;
-			is_neg = false;
-		} else if ((pIn3->flags & MEM_Real) == MEM_Real) {
-			if (pIn3->u.r > INT64_MAX)
-				i = INT64_MAX;
-			else if (pIn3->u.r < INT64_MIN)
-				i = INT64_MIN;
-			else
-				i = pIn3->u.r;
-			is_neg = i < 0;
-		} else {
-			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_to_diag_str(pIn3), "integer");
-			goto abort_due_to_error;
-		}
-		iKey = i;
-
-		/* If the P3 value could not be converted into an integer without
-		 * loss of information, then special processing is required...
-		 */
-		if ((pIn3->flags & (MEM_Int | MEM_UInt)) == 0) {
-			if ((pIn3->flags & MEM_Real)==0) {
-				/* If the P3 value cannot be converted into any kind of a number,
-				 * then the seek is not possible, so jump to P2
-				 */
-				VdbeBranchTaken(1,2); goto jump_to_p2;
-				break;
-			}
-
-			/* If the approximation iKey is larger than the actual real search
-			 * term, substitute >= for > and < for <=. e.g. if the search term
-			 * is 4.9 and the integer approximation 5:
-			 *
-			 *        (x >  4.9)    ->     (x >= 5)
-			 *        (x <= 4.9)    ->     (x <  5)
-			 */
-			if (pIn3->u.r<(double)iKey) {
-				assert(OP_SeekGE==(OP_SeekGT-1));
-				assert(OP_SeekLT==(OP_SeekLE-1));
-				assert((OP_SeekLE & 0x0001)==(OP_SeekGT & 0x0001));
-				if ((oc & 0x0001)==(OP_SeekGT & 0x0001)) oc--;
-			}
-
-			/* If the approximation iKey is smaller than the actual real search
-			 * term, substitute <= for < and > for >=.
-			 */
-			else if (pIn3->u.r>(double)iKey) {
-				assert(OP_SeekLE==(OP_SeekLT+1));
-				assert(OP_SeekGT==(OP_SeekGE+1));
-				assert((OP_SeekLT & 0x0001)==(OP_SeekGE & 0x0001));
-				if ((oc & 0x0001)==(OP_SeekLT & 0x0001)) oc++;
-			}
-		}
-	}
-skip_truncate:
 	/*
 	 * For a cursor with the OPFLAG_SEEKEQ hint, only the
 	 * OP_SeekGE and OP_SeekLE opcodes are allowed, and these
@@ -3593,9 +3500,6 @@ skip_truncate:
 	r.key_def = pC->key_def;
 	r.nField = (u16)nField;
 
-	if (int_field > 0)
-		mem_set_int(&aMem[int_field], iKey, is_neg);
-
 	r.default_rc = ((1 & (oc - OP_SeekLT)) ? -1 : +1);
 	assert(oc!=OP_SeekGT || r.default_rc==-1);
 	assert(oc!=OP_SeekLE || r.default_rc==-1);
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 6d8768865..97ba59151 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -910,10 +910,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about the W
 		enum field_type *end_types = NULL;
 		u8 bSeekPastNull = 0;	/* True to seek past initial nulls */
 		u8 bStopAtNull = 0;	/* Add condition to terminate at NULLs */
-		int force_integer_reg = -1;  /* If non-negative: number of
-					      * column which must be converted
-					      * to integer type, used for IPK.
-					      */
 
 		struct index_def *idx_def = pLoop->index_def;
 		assert(idx_def != NULL);
@@ -1120,21 +1116,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about the W
 							   1);
 			}
 		}
-		/* Inequality constraint comes always at the end of list. */
-		part_count = idx_def->key_def->part_count;
-		if (pRangeStart != NULL) {
-			/*
-			 * nEq == 0 means that filter condition
-			 * contains only inequality.
-			 */
-			uint32_t ineq_idx = nEq == 0 ? 0 : nEq - 1;
-			assert(ineq_idx < part_count);
-			enum field_type ineq_type =
-				idx_def->key_def->parts[ineq_idx].type;
-			if (ineq_type == FIELD_TYPE_INTEGER ||
-			    ineq_type == FIELD_TYPE_UNSIGNED)
-				force_integer_reg = regBase + nEq;
-		}
 		emit_apply_type(pParse, regBase, nConstraint - bSeekPastNull,
 				start_types);
 		if (pLoop->nSkip > 0 && nConstraint == pLoop->nSkip) {
@@ -1152,14 +1133,6 @@ sqlWhereCodeOneLoopStart(WhereInfo * pWInfo,	/* Complete information about the W
 			}
 			sqlVdbeAddOp4Int(v, op, iIdxCur, addrNxt, regBase,
 					     nConstraint);
-			/* If this is Seek* opcode, and IPK is detected in the
-			 * constraints vector: force it to be integer.
-			 */
-			if ((op == OP_SeekGE || op == OP_SeekGT
-			    || op == OP_SeekLE || op == OP_SeekLT)
-			    && force_integer_reg > 0) {
-				sqlVdbeChangeP5(v, force_integer_reg);
-			}
 			VdbeCoverage(v);
 			VdbeCoverageIf(v, op == OP_Rewind);
 			testcase(op == OP_Rewind);
-- 
2.25.1

  parent reply	other threads:[~2020-07-16 14:47 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-16 14:45 [Tarantool-patches] [PATCH v6 00/22] sql: change implicit cast imeevma
2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 01/22] sql: change implicit cast for assignment imeevma
2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 02/22] sql: use ApplyType to check function arguments imeevma
2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 03/22] sql: check args of abs() imeevma
2020-07-16 16:12   ` Nikita Pettik
2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 04/22] sql: check args of avg(), sum() and total() imeevma
2020-07-16 16:21   ` Nikita Pettik
2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 05/22] sql: check args of char() imeevma
2020-07-16 16:27   ` Nikita Pettik
2020-07-16 14:45 ` [Tarantool-patches] [PATCH v6 06/22] sql: check args of length() imeevma
2020-07-16 16:31   ` Nikita Pettik
2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 07/22] sql: check operands of LIKE imeevma
2020-07-16 17:03   ` Nikita Pettik
2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 08/22] sql: check args of lower() and upper() imeevma
2020-07-16 17:09   ` Nikita Pettik
2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 09/22] sql: check args of position() imeevma
2020-07-16 17:28   ` Nikita Pettik
2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 10/22] sql: check args of randomblob() imeevma
2020-07-16 17:28   ` Nikita Pettik
2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 11/22] sql: check args of replace() imeevma
2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 12/22] sql: check args of round() imeevma
2020-07-16 14:46 ` [Tarantool-patches] [PATCH v6 13/22] sql: check args of soundex() imeevma
2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 14/22] sql: check args of substr() imeevma
2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 15/22] sql: check args of unicode() imeevma
2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 16/22] sql: check args of zeroblob() imeevma
2020-07-16 14:47 ` imeevma [this message]
2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 18/22] sql: add implicit cast between numbers in OP_Seek* imeevma
2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 19/22] sql: change comparison between numbers using index imeevma
2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 20/22] sql: remove implicit cast from comparison opcodes imeevma
2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 21/22] sql: fix implicit cast in opcode MustBeInt imeevma
2020-07-16 14:47 ` [Tarantool-patches] [PATCH v6 22/22] sql: remove implicit cast from MakeRecord opcode imeevma

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=a25ad9c5dadec9a13cc35b10fcdcf16476dbc202.1594909974.git.imeevma@gmail.com \
    --to=imeevma@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=tsafin@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v6 17/22] sql: remove unused DOUBLE to INTEGER conversion' \
    /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