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 1/5] sql: always erase numeric flag after stringifying Date: Wed, 24 Jul 2019 14:42:43 +0300 [thread overview] Message-ID: <b9fe84f6d2df8a75df4c8f5e88f3f292b012a04d.1563967510.git.korablev@tarantool.org> (raw) In-Reply-To: <cover.1563967510.git.korablev@tarantool.org> In-Reply-To: <cover.1563967510.git.korablev@tarantool.org> Function which converts values to string representation (sqlVdbeMemStringify()) erase MEM_Int/MEM_Real/MEM_Bool flags only when it is specified by 'force' parameter. Hence, when 'force' argument is false, memory cell after conversion will contain string value, but flag indicating its type will be equal to combination of MEM_Str and one of mentioned flags. It seems to be remains of affinity routines, since in current state memory cell must have only one type. What is more, it can lead to unpredicted consequences, for instance assertion fault (sql_value_type() assumes that value has one specific type). Let's fix it removing 'force' argument from sqlVdbeMemStringify() and always clean up type flag. --- src/box/sql/vdbe.c | 8 ++++---- src/box/sql/vdbeInt.h | 2 +- src/box/sql/vdbemem.c | 9 +++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index c8887f9b7..9f3879910 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -183,7 +183,7 @@ vdbeTakeBranch(int iSrcLine, u8 I, u8 M) * already. Return non-zero if a malloc() fails. */ #define Stringify(P) \ - if(((P)->flags&(MEM_Str|MEM_Blob))==0 && sqlVdbeMemStringify(P,0)) \ + if(((P)->flags&(MEM_Str|MEM_Blob))==0 && sqlVdbeMemStringify(P)) \ { goto no_mem; } /* @@ -333,7 +333,7 @@ mem_apply_type(struct Mem *record, enum field_type type) */ if ((record->flags & MEM_Str) == 0) { if ((record->flags & (MEM_Real | MEM_Int))) - sqlVdbeMemStringify(record, 1); + sqlVdbeMemStringify(record); } record->flags &= ~(MEM_Real | MEM_Int); return 0; @@ -2144,7 +2144,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ if ((flags1 & MEM_Str)==0 && (flags1 & (MEM_Int|MEM_Real))!=0) { testcase( pIn1->flags & MEM_Int); testcase( pIn1->flags & MEM_Real); - sqlVdbeMemStringify(pIn1, 1); + sqlVdbeMemStringify(pIn1); testcase( (flags1&MEM_Dyn) != (pIn1->flags&MEM_Dyn)); flags1 = (pIn1->flags & ~MEM_TypeMask) | (flags1 & MEM_TypeMask); assert(pIn1!=pIn3); @@ -2152,7 +2152,7 @@ case OP_Ge: { /* same as TK_GE, jump, in1, in3 */ if ((flags3 & MEM_Str)==0 && (flags3 & (MEM_Int|MEM_Real))!=0) { testcase( pIn3->flags & MEM_Int); testcase( pIn3->flags & MEM_Real); - sqlVdbeMemStringify(pIn3, 1); + sqlVdbeMemStringify(pIn3); testcase( (flags3&MEM_Dyn) != (pIn3->flags&MEM_Dyn)); flags3 = (pIn3->flags & ~MEM_TypeMask) | (flags3 & MEM_TypeMask); } diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index 6bfeecc85..7d2470b07 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -469,7 +469,7 @@ void sqlVdbeMemInit(Mem *, sql *, u32); void sqlVdbeMemSetNull(Mem *); void sqlVdbeMemSetZeroBlob(Mem *, int); int sqlVdbeMemMakeWriteable(Mem *); -int sqlVdbeMemStringify(Mem *, u8); +int sqlVdbeMemStringify(Mem *); int sqlVdbeIntValue(Mem *, int64_t *); int sqlVdbeMemIntegerify(Mem *, bool is_forced); int sqlVdbeRealValue(Mem *, double *); diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c index 4e4bd597d..d85148bc3 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -275,7 +275,7 @@ sqlVdbeMemNulTerminate(Mem * pMem) * user and the latter is an internal programming error. */ int -sqlVdbeMemStringify(Mem * pMem, u8 bForce) +sqlVdbeMemStringify(Mem * pMem) { int fg = pMem->flags; const int nByte = 32; @@ -292,16 +292,17 @@ sqlVdbeMemStringify(Mem * pMem, u8 bForce) } if (fg & MEM_Int) { sql_snprintf(nByte, pMem->z, "%lld", pMem->u.i); + pMem->flags &= ~MEM_Int; } else if ((fg & MEM_Bool) != 0) { sql_snprintf(nByte, pMem->z, "%s", pMem->u.b ? "true" : "false"); + pMem->flags &= ~MEM_Bool; } else { assert(fg & MEM_Real); sql_snprintf(nByte, pMem->z, "%!.15g", pMem->u.r); + pMem->flags &= ~MEM_Real; } pMem->n = sqlStrlen30(pMem->z); pMem->flags |= MEM_Str | MEM_Term; - if (bForce) - pMem->flags &= ~(MEM_Int | MEM_Real); return 0; } @@ -1107,7 +1108,7 @@ valueToText(sql_value * pVal) pVal->flags |= MEM_Str; sqlVdbeMemNulTerminate(pVal); /* IMP: R-31275-44060 */ } else { - sqlVdbeMemStringify(pVal, 0); + sqlVdbeMemStringify(pVal); assert(0 == (1 & SQL_PTR_TO_INT(pVal->z))); } return pVal->z; -- 2.15.1
next prev parent reply other threads:[~2019-07-24 11:42 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-24 11:42 [tarantool-patches] [PATCH 0/5] Introduce VARBINARY in SQL Nikita Pettik 2019-07-24 11:42 ` Nikita Pettik [this message] 2019-07-24 11:42 ` [tarantool-patches] [PATCH 2/5] sql: fix resulting type calculation for CASE-WHEN stmt Nikita Pettik 2019-07-25 22:12 ` [tarantool-patches] " Vladislav Shpilevoy [not found] ` <a061e845-eeb1-00d1-9141-3b9bb87768f5@tarantool.org> 2019-07-28 23:56 ` n.pettik 2019-07-24 11:42 ` [tarantool-patches] [PATCH 3/5] sql: use 'varbinary' as a name of type instead of 'blob' Nikita Pettik 2019-07-25 22:11 ` [tarantool-patches] " Vladislav Shpilevoy [not found] ` <2e655514-0fec-8baf-20a8-d49e5586b047@tarantool.org> 2019-07-28 23:56 ` n.pettik 2019-07-29 21:03 ` Vladislav Shpilevoy 2019-07-30 13:43 ` n.pettik 2019-07-24 11:42 ` [tarantool-patches] [PATCH 4/5] sql: make built-ins raise errors for varbin args Nikita Pettik 2019-07-25 22:11 ` [tarantool-patches] " Vladislav Shpilevoy [not found] ` <05d15035-2552-1f05-b7ce-facfbbc3a520@tarantool.org> 2019-07-28 23:59 ` n.pettik 2019-07-24 11:42 ` [tarantool-patches] [PATCH 5/5] sql: introduce VARBINARY column type Nikita Pettik 2019-07-25 22:12 ` [tarantool-patches] " Vladislav Shpilevoy [not found] ` <49a188eb-dafe-44e7-a0fd-e9244b68e721@tarantool.org> 2019-07-29 0:03 ` n.pettik 2019-07-29 20:55 ` Vladislav Shpilevoy 2019-07-30 13:44 ` n.pettik 2019-07-30 19:41 ` Vladislav Shpilevoy 2019-07-30 19:52 ` Vladislav Shpilevoy 2019-07-31 14:51 ` n.pettik 2019-08-01 8:42 ` [tarantool-patches] Re: [PATCH 0/5] Introduce VARBINARY in SQL Kirill Yukhin
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=b9fe84f6d2df8a75df4c8f5e88f3f292b012a04d.1563967510.git.korablev@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [tarantool-patches] [PATCH 1/5] sql: always erase numeric flag after stringifying' \ /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