Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] sql: print space names in VDBE dump
@ 2018-05-17 13:25 Kirill Yukhin
  2018-05-17 15:12 ` [tarantool-patches] " Vladislav Shpilevoy
  0 siblings, 1 reply; 5+ messages in thread
From: Kirill Yukhin @ 2018-05-17 13:25 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches, Kirill Yukhin

Before the patch, when LoadPtr instruction is dumped,
raw pointer value was printed, which was looked like a
garbade. The patch inroduces new pointer type: space ptr,
which allows to print space name during VDBE dump.
---
 src/box/sql/analyze.c |  4 ++--
 src/box/sql/build.c   |  3 +--
 src/box/sql/insert.c  |  2 +-
 src/box/sql/vdbe.c    |  6 +++---
 src/box/sql/vdbe.h    | 19 ++++++++++++++++++-
 src/box/sql/vdbeInt.h |  3 ++-
 src/box/sql/vdbeaux.c |  9 +++++++--
 7 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 50d212a..563cd18 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -910,8 +910,8 @@ analyzeOneTable(Parse * pParse,	/* Parser context */
 		struct space *space =
 			space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
 		assert(space != NULL);
-		sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
-				     (void *) space);
+		sql_vdbe_add_op4_spaceptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
+					  space);
 		sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, pIdx->tnum,
 				  space_ptr_reg);
 		sql_vdbe_set_p4_key_def(pParse, pIdx);
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index c96d351..1a800fd 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1222,8 +1222,7 @@ emit_open_cursor(Parse *parse_context, int cursor, int entity_id)
 	assert(space != NULL);
 	Vdbe *vdbe = parse_context->pVdbe;
 	int space_ptr_reg = ++parse_context->nMem;
-	sqlite3VdbeAddOp4Ptr(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0,
-			     (void *) space);
+	sql_vdbe_add_op4_spaceptr(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, space);
 	return sqlite3VdbeAddOp3(vdbe, OP_OpenWrite, cursor, entity_id,
 				 space_ptr_reg);
 }
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 22130ef..7ea0f52 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -1613,7 +1613,7 @@ sqlite3OpenTableAndIndices(Parse * pParse,	/* Parsing context */
 	struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pTab->tnum));
 	assert(space != NULL);
 	int space_ptr_reg = ++pParse->nMem;
-	sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0, (void *) space);
+	sql_vdbe_add_op4_spaceptr(v, OP_LoadPtr, 0, space_ptr_reg, 0, space);
 
 	/* One iteration of this cycle adds OpenRead/OpenWrite which
 	 * opens cursor for current index.
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index b4b59da..e27fe2f 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1074,12 +1074,12 @@ case OP_Int64: {           /* out2 */
 /* Opcode: LoadPtr * P2 * P4 *
  * Synopsis: r[P2] = P4
  *
- * P4 is a generic pointer. Copy it into register P2.
+ * P4 is a generic or space pointer. Copy it into register P2.
  */
 case OP_LoadPtr: {
 	pOut = out2Prerelease(p, pOp);
-	assert(pOp->p4type == P4_PTR);
-	pOut->u.p = pOp->p4.p;
+	assert(pOp->p4type == P4_PTR || pOp->p4type == P4_SPACEPTR );
+	pOut->u.p = pOp->p4.space;
 	pOut->flags = MEM_Ptr;
 	break;
 }
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index 39f8da4..bc54efc 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -86,6 +86,8 @@ struct VdbeOp {
 		int (*xAdvance) (BtCursor *, int *);
 		/** Used when p4type is P4_KEYDEF. */
 		struct key_def *key_def;
+		/** Used when p4type is P4_SPACEPTR. */
+		struct space *space;
 	} p4;
 #ifdef SQLITE_ENABLE_EXPLAIN_COMMENTS
 	char *zComment;		/* Comment to improve readability */
@@ -146,6 +148,7 @@ typedef struct VdbeOpList VdbeOpList;
 #define P4_BOOL     (-17)	/* P4 is a bool value */
 #define P4_PTR      (-18)	/* P4 is a generic pointer */
 #define P4_KEYDEF   (-19)       /* P4 is a pointer to key_def structure. */
+#define P4_SPACEPTR (-20)       /* P4 is a space pointer */
 
 /* Error message codes for OP_Halt */
 #define P5_ConstraintNotNull 1
@@ -224,7 +227,21 @@ int sqlite3VdbeAddOp3(Vdbe *, int, int, int, int);
 int sqlite3VdbeAddOp4(Vdbe *, int, int, int, int, const char *zP4, int);
 int sqlite3VdbeAddOp4Dup8(Vdbe *, int, int, int, int, const u8 *, int);
 int sqlite3VdbeAddOp4Int(Vdbe *, int, int, int, int, int);
-int sqlite3VdbeAddOp4Ptr(Vdbe *, int, int, int, int, void *);
+/**
+ * Add new 4-arg instruction to VDBE program. 4th argument is a
+ * space pointer.
+ *
+ * @param v VDBE program being built.
+ * @param op Opcode for new instruction.
+ * @param op1 1st operand.
+ * @param op2 2nd operand.
+ * @param op3 3rd operand.
+ * @param op4 4th operand. Must be space pointer.
+ * @retval VDBE address of the instruction added.
+ */
+int
+sql_vdbe_add_op4_spaceptr(struct Vdbe *v, int op, int op1, int op2, int op3,
+			  struct space *op4);
 void sqlite3VdbeEndCoroutine(Vdbe *, int);
 #if defined(SQLITE_DEBUG) && !defined(SQLITE_TEST_REALLOC_STRESS)
 void sqlite3VdbeVerifyNoMallocRequired(Vdbe * p, int N);
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 884963f..d067ec0 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -235,7 +235,8 @@ struct Mem {
 #define MEM_Frame     0x0080	/* Value is a VdbeFrame object */
 #define MEM_Undefined 0x0100	/* Value is undefined */
 #define MEM_Cleared   0x0200	/* NULL set by OP_Null, not from data */
-#define MEM_TypeMask  0x83ff	/* Mask of type bits */
+#define MEM_SpacePtr  0x0400    /* Value is a space pointer */
+#define MEM_TypeMask  0x87ff	/* Mask of type bits */
 
 /* Whenever Mem contains a valid string or blob representation, one of
  * the following flags must be set to determine the memory management
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index e67fcae..cbe066a 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -445,11 +445,12 @@ sqlite3VdbeAddOp4Int(Vdbe * p,	/* Add the opcode to this VM */
 }
 
 int
-sqlite3VdbeAddOp4Ptr(Vdbe *p, int op, int p1, int p2, int p3, void *ptr)
+sql_vdbe_add_op4_spaceptr(struct Vdbe *p, int op, int p1, int p2, int p3,
+			  struct space *ptr)
 {
 	int addr = sqlite3VdbeAddOp3(p, op, p1, p2, p3);
 	VdbeOp *pOp = &p->aOp[addr];
-	pOp->p4type = P4_PTR;
+	pOp->p4type = P4_SPACEPTR;
 	pOp->p4.p = ptr;
 	return addr;
 }
@@ -1638,6 +1639,10 @@ displayP4(Op * pOp, char *zTemp, int nTemp)
 			zTemp[0] = 0;
 			break;
 		}
+	case P4_SPACEPTR: {
+		sqlite3XPrintf(&x, "space<name=%s>", space_name(pOp->p4.space));
+		break;
+	}
 	default:{
 			zP4 = pOp->p4.z;
 			if (zP4 == 0) {
-- 
2.16.2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: print space names in VDBE dump
  2018-05-17 13:25 [tarantool-patches] [PATCH] sql: print space names in VDBE dump Kirill Yukhin
@ 2018-05-17 15:12 ` Vladislav Shpilevoy
  2018-05-17 16:01   ` Kirill Yukhin
  0 siblings, 1 reply; 5+ messages in thread
From: Vladislav Shpilevoy @ 2018-05-17 15:12 UTC (permalink / raw)
  To: tarantool-patches, Kirill Yukhin

Hello. Thanks for the patch! See 3 comments below.

On 17/05/2018 16:25, Kirill Yukhin wrote:
> Before the patch, when LoadPtr instruction is dumped,
> raw pointer value was printed, which was looked like a
> garbade. The patch inroduces new pointer type: space ptr,
> which allows to print space name during VDBE dump.
> ---

1. Please, post here branch and issue link (if issue exists).

2. Can not find any similar branch on github by kyukhin/ prefix.

>   src/box/sql/analyze.c |  4 ++--
>   src/box/sql/build.c   |  3 +--
>   src/box/sql/insert.c  |  2 +-
>   src/box/sql/vdbe.c    |  6 +++---
>   src/box/sql/vdbe.h    | 19 ++++++++++++++++++-
>   src/box/sql/vdbeInt.h |  3 ++-
>   src/box/sql/vdbeaux.c |  9 +++++++--
>   7 files changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index 50d212a..563cd18 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -910,8 +910,8 @@ analyzeOneTable(Parse * pParse,	/* Parser context */
>   		struct space *space =
>   			space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
>   		assert(space != NULL);
> -		sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
> -				     (void *) space);
> +		sql_vdbe_add_op4_spaceptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
> +					  space);

3. I think, all VdbeAppOp functions family must be refactored at once.
You can introduce P4_SPACEPTR and use
sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, space, P4_SPACEPTR);.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: print space names in VDBE dump
  2018-05-17 15:12 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-05-17 16:01   ` Kirill Yukhin
  2018-05-17 16:13     ` Vladislav Shpilevoy
  0 siblings, 1 reply; 5+ messages in thread
From: Kirill Yukhin @ 2018-05-17 16:01 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

Hello Vlad,
Thanks for review.

Issue: n/a
Branch: https://github.com/tarantool/tarantool/tree/kyukhin/human-readable-s-name-in-trace

My comments are inlined. Updated patch is in the bottom.

On 17 мая 18:12, Vladislav Shpilevoy wrote:
> Hello. Thanks for the patch! See 3 comments below.
> 
> On 17/05/2018 16:25, Kirill Yukhin wrote:
> > Before the patch, when LoadPtr instruction is dumped,
> > raw pointer value was printed, which was looked like a
> > garbade. The patch inroduces new pointer type: space ptr,
> > which allows to print space name during VDBE dump.
> > ---
> 
> 1. Please, post here branch and issue link (if issue exists).
Done.

> 2. Can not find any similar branch on github by kyukhin/ prefix.
Pushed.

> > -		sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
> > -				     (void *) space);
> > +		sql_vdbe_add_op4_spaceptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
> > +					  space);
> 
> 3. I think, all VdbeAppOp functions family must be refactored at once.
> You can introduce P4_SPACEPTR and use
> sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, space, P4_SPACEPTR);.
Fixed. New function removed.

--
Regards, Kirill Yukhin

diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 50d212a..998dd5b 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -910,8 +910,8 @@ analyzeOneTable(Parse * pParse,	/* Parser context */
 		struct space *space =
 			space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
 		assert(space != NULL);
-		sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
-				     (void *) space);
+		sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
+				  (void*)space, P4_SPACEPTR);
 		sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, pIdx->tnum,
 				  space_ptr_reg);
 		sql_vdbe_set_p4_key_def(pParse, pIdx);
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index c96d351..d39f110 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1222,8 +1222,8 @@ emit_open_cursor(Parse *parse_context, int cursor, int entity_id)
 	assert(space != NULL);
 	Vdbe *vdbe = parse_context->pVdbe;
 	int space_ptr_reg = ++parse_context->nMem;
-	sqlite3VdbeAddOp4Ptr(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0,
-			     (void *) space);
+	sqlite3VdbeAddOp4(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
+			  P4_SPACEPTR);
 	return sqlite3VdbeAddOp3(vdbe, OP_OpenWrite, cursor, entity_id,
 				 space_ptr_reg);
 }
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 22130ef..5ada87e 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -1613,7 +1613,8 @@ sqlite3OpenTableAndIndices(Parse * pParse,	/* Parsing context */
 	struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pTab->tnum));
 	assert(space != NULL);
 	int space_ptr_reg = ++pParse->nMem;
-	sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0, (void *) space);
+	sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
+			  P4_SPACEPTR);
 
 	/* One iteration of this cycle adds OpenRead/OpenWrite which
 	 * opens cursor for current index.
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index b4b59da..e27fe2f 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1074,12 +1074,12 @@ case OP_Int64: {           /* out2 */
 /* Opcode: LoadPtr * P2 * P4 *
  * Synopsis: r[P2] = P4
  *
- * P4 is a generic pointer. Copy it into register P2.
+ * P4 is a generic or space pointer. Copy it into register P2.
  */
 case OP_LoadPtr: {
 	pOut = out2Prerelease(p, pOp);
-	assert(pOp->p4type == P4_PTR);
-	pOut->u.p = pOp->p4.p;
+	assert(pOp->p4type == P4_PTR || pOp->p4type == P4_SPACEPTR );
+	pOut->u.p = pOp->p4.space;
 	pOut->flags = MEM_Ptr;
 	break;
 }
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index 39f8da4..f6dad22 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -86,6 +86,8 @@ struct VdbeOp {
 		int (*xAdvance) (BtCursor *, int *);
 		/** Used when p4type is P4_KEYDEF. */
 		struct key_def *key_def;
+		/** Used when p4type is P4_SPACEPTR. */
+		struct space *space;
 	} p4;
 #ifdef SQLITE_ENABLE_EXPLAIN_COMMENTS
 	char *zComment;		/* Comment to improve readability */
@@ -146,6 +148,7 @@ typedef struct VdbeOpList VdbeOpList;
 #define P4_BOOL     (-17)	/* P4 is a bool value */
 #define P4_PTR      (-18)	/* P4 is a generic pointer */
 #define P4_KEYDEF   (-19)       /* P4 is a pointer to key_def structure. */
+#define P4_SPACEPTR (-20)       /* P4 is a space pointer */
 
 /* Error message codes for OP_Halt */
 #define P5_ConstraintNotNull 1
@@ -224,7 +227,6 @@ int sqlite3VdbeAddOp3(Vdbe *, int, int, int, int);
 int sqlite3VdbeAddOp4(Vdbe *, int, int, int, int, const char *zP4, int);
 int sqlite3VdbeAddOp4Dup8(Vdbe *, int, int, int, int, const u8 *, int);
 int sqlite3VdbeAddOp4Int(Vdbe *, int, int, int, int, int);
-int sqlite3VdbeAddOp4Ptr(Vdbe *, int, int, int, int, void *);
 void sqlite3VdbeEndCoroutine(Vdbe *, int);
 #if defined(SQLITE_DEBUG) && !defined(SQLITE_TEST_REALLOC_STRESS)
 void sqlite3VdbeVerifyNoMallocRequired(Vdbe * p, int N);
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index e67fcae..1304149 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -444,16 +444,6 @@ sqlite3VdbeAddOp4Int(Vdbe * p,	/* Add the opcode to this VM */
 	return addr;
 }
 
-int
-sqlite3VdbeAddOp4Ptr(Vdbe *p, int op, int p1, int p2, int p3, void *ptr)
-{
-	int addr = sqlite3VdbeAddOp3(p, op, p1, p2, p3);
-	VdbeOp *pOp = &p->aOp[addr];
-	pOp->p4type = P4_PTR;
-	pOp->p4.p = ptr;
-	return addr;
-}
-
 /* Insert the end of a co-routine
  */
 void
@@ -1638,6 +1628,10 @@ displayP4(Op * pOp, char *zTemp, int nTemp)
 			zTemp[0] = 0;
 			break;
 		}
+	case P4_SPACEPTR: {
+		sqlite3XPrintf(&x, "space<name=%s>", space_name(pOp->p4.space));
+		break;
+	}
 	default:{
 			zP4 = pOp->p4.z;
 			if (zP4 == 0) {

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: print space names in VDBE dump
  2018-05-17 16:01   ` Kirill Yukhin
@ 2018-05-17 16:13     ` Vladislav Shpilevoy
  2018-05-17 16:34       ` Kirill Yukhin
  0 siblings, 1 reply; 5+ messages in thread
From: Vladislav Shpilevoy @ 2018-05-17 16:13 UTC (permalink / raw)
  To: Kirill Yukhin; +Cc: tarantool-patches

LGTM.

On 17/05/2018 19:01, Kirill Yukhin wrote:
> Hello Vlad,
> Thanks for review.
> 
> Issue: n/a
> Branch: https://github.com/tarantool/tarantool/tree/kyukhin/human-readable-s-name-in-trace
> 
> My comments are inlined. Updated patch is in the bottom.
> 
> On 17 мая 18:12, Vladislav Shpilevoy wrote:
>> Hello. Thanks for the patch! See 3 comments below.
>>
>> On 17/05/2018 16:25, Kirill Yukhin wrote:
>>> Before the patch, when LoadPtr instruction is dumped,
>>> raw pointer value was printed, which was looked like a
>>> garbade. The patch inroduces new pointer type: space ptr,
>>> which allows to print space name during VDBE dump.
>>> ---
>>
>> 1. Please, post here branch and issue link (if issue exists).
> Done.
> 
>> 2. Can not find any similar branch on github by kyukhin/ prefix.
> Pushed.
> 
>>> -		sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
>>> -				     (void *) space);
>>> +		sql_vdbe_add_op4_spaceptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
>>> +					  space);
>>
>> 3. I think, all VdbeAppOp functions family must be refactored at once.
>> You can introduce P4_SPACEPTR and use
>> sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, space, P4_SPACEPTR);.
> Fixed. New function removed.
> 
> --
> Regards, Kirill Yukhin
> 
> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index 50d212a..998dd5b 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -910,8 +910,8 @@ analyzeOneTable(Parse * pParse,	/* Parser context */
>   		struct space *space =
>   			space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
>   		assert(space != NULL);
> -		sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
> -				     (void *) space);
> +		sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0,
> +				  (void*)space, P4_SPACEPTR);
>   		sqlite3VdbeAddOp3(v, OP_OpenRead, iIdxCur, pIdx->tnum,
>   				  space_ptr_reg);
>   		sql_vdbe_set_p4_key_def(pParse, pIdx);
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index c96d351..d39f110 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -1222,8 +1222,8 @@ emit_open_cursor(Parse *parse_context, int cursor, int entity_id)
>   	assert(space != NULL);
>   	Vdbe *vdbe = parse_context->pVdbe;
>   	int space_ptr_reg = ++parse_context->nMem;
> -	sqlite3VdbeAddOp4Ptr(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0,
> -			     (void *) space);
> +	sqlite3VdbeAddOp4(vdbe, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
> +			  P4_SPACEPTR);
>   	return sqlite3VdbeAddOp3(vdbe, OP_OpenWrite, cursor, entity_id,
>   				 space_ptr_reg);
>   }
> diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
> index 22130ef..5ada87e 100644
> --- a/src/box/sql/insert.c
> +++ b/src/box/sql/insert.c
> @@ -1613,7 +1613,8 @@ sqlite3OpenTableAndIndices(Parse * pParse,	/* Parsing context */
>   	struct space *space = space_by_id(SQLITE_PAGENO_TO_SPACEID(pTab->tnum));
>   	assert(space != NULL);
>   	int space_ptr_reg = ++pParse->nMem;
> -	sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0, (void *) space);
> +	sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, (void*)space,
> +			  P4_SPACEPTR);
>   
>   	/* One iteration of this cycle adds OpenRead/OpenWrite which
>   	 * opens cursor for current index.
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index b4b59da..e27fe2f 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -1074,12 +1074,12 @@ case OP_Int64: {           /* out2 */
>   /* Opcode: LoadPtr * P2 * P4 *
>    * Synopsis: r[P2] = P4
>    *
> - * P4 is a generic pointer. Copy it into register P2.
> + * P4 is a generic or space pointer. Copy it into register P2.
>    */
>   case OP_LoadPtr: {
>   	pOut = out2Prerelease(p, pOp);
> -	assert(pOp->p4type == P4_PTR);
> -	pOut->u.p = pOp->p4.p;
> +	assert(pOp->p4type == P4_PTR || pOp->p4type == P4_SPACEPTR );
> +	pOut->u.p = pOp->p4.space;
>   	pOut->flags = MEM_Ptr;
>   	break;
>   }
> diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
> index 39f8da4..f6dad22 100644
> --- a/src/box/sql/vdbe.h
> +++ b/src/box/sql/vdbe.h
> @@ -86,6 +86,8 @@ struct VdbeOp {
>   		int (*xAdvance) (BtCursor *, int *);
>   		/** Used when p4type is P4_KEYDEF. */
>   		struct key_def *key_def;
> +		/** Used when p4type is P4_SPACEPTR. */
> +		struct space *space;
>   	} p4;
>   #ifdef SQLITE_ENABLE_EXPLAIN_COMMENTS
>   	char *zComment;		/* Comment to improve readability */
> @@ -146,6 +148,7 @@ typedef struct VdbeOpList VdbeOpList;
>   #define P4_BOOL     (-17)	/* P4 is a bool value */
>   #define P4_PTR      (-18)	/* P4 is a generic pointer */
>   #define P4_KEYDEF   (-19)       /* P4 is a pointer to key_def structure. */
> +#define P4_SPACEPTR (-20)       /* P4 is a space pointer */
>   
>   /* Error message codes for OP_Halt */
>   #define P5_ConstraintNotNull 1
> @@ -224,7 +227,6 @@ int sqlite3VdbeAddOp3(Vdbe *, int, int, int, int);
>   int sqlite3VdbeAddOp4(Vdbe *, int, int, int, int, const char *zP4, int);
>   int sqlite3VdbeAddOp4Dup8(Vdbe *, int, int, int, int, const u8 *, int);
>   int sqlite3VdbeAddOp4Int(Vdbe *, int, int, int, int, int);
> -int sqlite3VdbeAddOp4Ptr(Vdbe *, int, int, int, int, void *);
>   void sqlite3VdbeEndCoroutine(Vdbe *, int);
>   #if defined(SQLITE_DEBUG) && !defined(SQLITE_TEST_REALLOC_STRESS)
>   void sqlite3VdbeVerifyNoMallocRequired(Vdbe * p, int N);
> diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
> index e67fcae..1304149 100644
> --- a/src/box/sql/vdbeaux.c
> +++ b/src/box/sql/vdbeaux.c
> @@ -444,16 +444,6 @@ sqlite3VdbeAddOp4Int(Vdbe * p,	/* Add the opcode to this VM */
>   	return addr;
>   }
>   
> -int
> -sqlite3VdbeAddOp4Ptr(Vdbe *p, int op, int p1, int p2, int p3, void *ptr)
> -{
> -	int addr = sqlite3VdbeAddOp3(p, op, p1, p2, p3);
> -	VdbeOp *pOp = &p->aOp[addr];
> -	pOp->p4type = P4_PTR;
> -	pOp->p4.p = ptr;
> -	return addr;
> -}
> -
>   /* Insert the end of a co-routine
>    */
>   void
> @@ -1638,6 +1628,10 @@ displayP4(Op * pOp, char *zTemp, int nTemp)
>   			zTemp[0] = 0;
>   			break;
>   		}
> +	case P4_SPACEPTR: {
> +		sqlite3XPrintf(&x, "space<name=%s>", space_name(pOp->p4.space));
> +		break;
> +	}
>   	default:{
>   			zP4 = pOp->p4.z;
>   			if (zP4 == 0) {
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: print space names in VDBE dump
  2018-05-17 16:13     ` Vladislav Shpilevoy
@ 2018-05-17 16:34       ` Kirill Yukhin
  0 siblings, 0 replies; 5+ messages in thread
From: Kirill Yukhin @ 2018-05-17 16:34 UTC (permalink / raw)
  To: Vladislav Shpilevoy; +Cc: tarantool-patches

On 17 мая 19:13, Vladislav Shpilevoy wrote:
> LGTM.
Thanks! I've committed the patch to 2.0 branch.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-05-17 16:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-17 13:25 [tarantool-patches] [PATCH] sql: print space names in VDBE dump Kirill Yukhin
2018-05-17 15:12 ` [tarantool-patches] " Vladislav Shpilevoy
2018-05-17 16:01   ` Kirill Yukhin
2018-05-17 16:13     ` Vladislav Shpilevoy
2018-05-17 16:34       ` Kirill Yukhin

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