From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH 4/6] sql: enforce implicit type conversions
Date: Wed, 24 Oct 2018 02:28:24 +0300 [thread overview]
Message-ID: <E2586704-DF9B-4D7D-9E01-F1D20B8A6B6F@tarantool.org> (raw)
In-Reply-To: <999eb301-5c83-f1d8-491d-87c52ab5815f@tarantool.org>
> Also, please, elaborate what is wrong with tests - they do
> not pass.
Tests don’t pass due to forced conversion to int by unary plus..
So, if you don’t mind, I will simply change order of patches:
see git log.
> On 12/10/2018 14:19, n.pettik wrote:
>>>> From: Georgy Kirichenko <georgy@tarantool.org>
>>>> Most DBs (at least PostgreSQL, Oracle and DB2) allow to process
>>>> following queries:
>>>> CREATE TABLE t1 (id INT PRIMARY KEY);
>>>> INSERT INTO t1 VALUES (1.123), ('2');
>>>> In this particular case, 1.123 should be simply truncated to 1,
>>>> and '2' - converted to literal number 2.
>>>> After passing real type to Tarantool (instead of <SCALAR>), example
>>>> above would fail without conversions. Thus, lets add implicit
>>>> conversions inside VDBE to make this example be legal.
>>>> However, still some types conversions must be prohibited. For instance,
>>>> <BLOB> can't be converted to integer or floating point numerical,
>>>> and vice versa.
>>>
>>> As I see in the tests that it looks weird now:
>>>
>>> I can insert into 'int' a 'float' value, but can not
>>> compare them:
>>>
>>> box.sql.execute("SELECT bar, foo, 42, 'awesome' FROM foobar WHERE foo<2.001")
>>> ---
>>> -- error: Can't convert 2.001 to INTEGER
>>> ...
>>>
>>> Why? We should either forbid insertion and comparison, or
>>> allow both of them.
>> Well, now I agree that it looks quite strange, but I can't tell you
>> why I did so. It took quite a long to fix that, but workaround turned
>> out the be trivial:
>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>> index 3d2324867..827811cd1 100644
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -335,6 +335,14 @@ mem_apply_affinity(struct Mem *record, enum affinity_type affinity)
>> case AFFINITY_INTEGER:
>> if ((record->flags & MEM_Int) == MEM_Int)
>> return 0;
>> + if ((record->flags & MEM_Real) == MEM_Real) {
>> + int64_t i = (int64_t) record->u.r;
>> + if (i == record->u.r) {
>> + record->u.i = i;
>> + MemSetTypeFlag(record, MEM_Int);
>> + }
>> + return 0;
>> + }
>> return sqlite3VdbeMemIntegerify(record, false);
>> case AFFINITY_REAL:
>> if ((record->flags & MEM_Real) == MEM_Real)
>> @@ -1918,6 +1926,13 @@ case OP_MustBeInt: { /* jump, in1 */
>> pIn1 = &aMem[pOp->p1];
>> if ((pIn1->flags & MEM_Int)==0) {
>> mem_apply_affinity(pIn1, AFFINITY_INTEGER);
>> + if ((pIn1->flags & MEM_Real) == MEM_Real) {
>> + int64_t i = (int64_t) pIn1->u.r;
>> + if (i == pIn1->u.r) {
>> + pIn1->u.i = i;
>> + MemSetTypeFlag(pIn1, MEM_Int);
>> + }
>> + }
>
> 1. Why? mem_apply_affinity does the same and is called one line
> above, it is not?
I missed this moment. Removed redundant check:
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 8316c0a92..87972d1af 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1926,13 +1926,6 @@ case OP_MustBeInt: { /* jump, in1 */
pIn1 = &aMem[pOp->p1];
if ((pIn1->flags & MEM_Int)==0) {
mem_apply_affinity(pIn1, AFFINITY_INTEGER);
- if ((pIn1->flags & MEM_Real) == MEM_Real) {
- int64_t i = (int64_t) pIn1->u.r;
- if (i == pIn1->u.r) {
- pIn1->u.i = i;
- MemSetTypeFlag(pIn1, MEM_Int);
- }
- }
>
>> VdbeBranchTaken((pIn1->flags&MEM_Int)==0, 2);
>> if ((pIn1->flags & MEM_Int)==0) {
>> if (pOp->p2==0) {
>> @@ -3463,7 +3478,6 @@ case OP_SeekGT: { /* jump, in3 */
>> reg_ipk = pOp->p5;
>> if (reg_ipk > 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.
>> @@ -3473,9 +3487,18 @@ case OP_SeekGT: { /* jump, in3 */
>> applyNumericAffinity(pIn3, 0);
>> }
>> int64_t i;
>> - if (sqlite3VdbeIntValue(pIn3, &i) != 0) {
>> + if ((pIn3->flags & MEM_Int) == MEM_Int) {
>> + i = pIn3->u.i;
>> + } 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;
>
> 2. In the explanation below you say 'without loses' but here
> they are, it is not? An example: I do search by SeekLT with
> something bigger than INT64_MAX and the index really has
> INT64_MAX. You will turn this query into SeekLT by INT64_MAX,
> so INT64_MAX will not be presented in the result, but should be.
> I guess, if I am right, you can fix it easy just changing
> OP_SeekLT to OP_SeekLE in such a case, and the same for
> OP_SeekGT/GE and INT64_MIN.
In fact, your explanation is how it works now. Look below:
we save in iKey truncated value (e.g. INT64_MAX).
Then, we are comparing in with initial value:
/* 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++;
}
So, if it is smaller, we change opcode SeekLT to SeekLE
(SeekLE == SeekLT + 1). The same for reversed case.
Hence, you are right, but it already works this way.
>
>> + else
>> + i = pIn3->u.r;
>> + } else {
>> *Explanation*
>> Now OP_Affinity doesn’t ‘integrify’ float since it leads to losing
>> information concerning the fact that initial ‘real’ value was greater or less
>> than truncated ‘int’. Instead, it is done by explicit OP_MustBeInt conversion
>> OR when we are already in comparing routine and do it implicitly.
>> The only one case when OP_Affinity changes float to int is situation
>> when float can be casted to int without loses.
>>>> +
>>>> + sqlite3VdbeError(p, format, sqlite3_value_text(pIn1));
>>>> + goto abort_due_to_error;
>>>> + }
>>>> break;
>>>> }
>>>> #endif /* SQLITE_OMIT_CAST */
>>>> @@ -2451,11 +2516,13 @@ case OP_IfNot: { /* jump, in1 */
>>>> if (pIn1->flags & MEM_Null) {
>>>> c = pOp->p3;
>>>> } else {
>>>> -#ifdef SQLITE_OMIT_FLOATING_POINT
>>>> - c = sqlite3VdbeIntValue(pIn1)!=0;
>>>> -#else
>>>> - c = sqlite3VdbeRealValue(pIn1)!=0.0;
>>>> -#endif
>>>> + double v;
>>>> + if ((rc = sqlite3VdbeRealValue(pIn1, &v))) {
>>>> + sqlite3VdbeError(p, "Can't convert to numeric %s",
>>>
>>> 3. Why numeric? Maybe real?
>> It doesn’t really matter. Currently they are the same.
>
> 3. I know, but when a user asks to cast to real or just operates
> on real, and gets an error message "Can't convert to numeric",
> it is strange.
Ok, I don’t mind to fix it:
@@ -2535,7 +2528,7 @@ case OP_IfNot: { /* jump, in1 */
double v;
if (sqlite3VdbeRealValue(pIn1, &v) != 0) {
diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
- sqlite3_value_text(pIn1), "numeric");
+ sqlite3_value_text(pIn1), "real");
rc = SQL_TARANTOOL_ERROR;
goto abort_
>
>> diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
>> index 072a05066..f9527b650 100644
>> --- a/src/box/sql/vdbemem.c
>> +++ b/src/box/sql/vdbemem.c
>> /*
>> * Convert pMem to type integer. Invalidate any prior representations.
>> */
>> int
>> -sqlite3VdbeMemIntegerify(Mem * pMem)
>> +sqlite3VdbeMemIntegerify(Mem * pMem, bool is_forced)
>> {
>> assert(EIGHT_BYTE_ALIGNMENT(pMem));
>> - pMem->u.i = sqlite3VdbeIntValue(pMem);
>> + int64_t i;
>> + if (sqlite3VdbeIntValue(pMem, &i) == 0) {
>> + pMem->u.i = i;
>> + MemSetTypeFlag(pMem, MEM_Int);
>> + return SQLITE_OK;
>> + } else if ((pMem->flags & MEM_Real) != 0 && is_forced) {
>> + pMem->u.i = (int) pMem->u.r;
>> + MemSetTypeFlag(pMem, MEM_Int);
>> + return 0;
>
> 4. Lets do not mix SQLITE_OK/0 in one function.
Yep, surely. I see you’ve already fixed that, so skipped.
>
>> + }
>> +
>> + double d;
>> + if (sqlite3VdbeRealValue(pMem, &d) || (int64_t) d != d) {
>> + return SQLITE_ERROR;
>> + }
>> + pMem->u.i = (int64_t) d;
>> MemSetTypeFlag(pMem, MEM_Int);
>> return SQLITE_OK;
>> }
>> @@ -583,46 +601,47 @@ sqlite3VdbeMemNumerify(Mem * pMem)
>> * affinity even if that results in loss of data. This routine is
>> * used (for example) to implement the SQL "cast()" operator.
>> */
>> -void
>> +int
>> sqlite3VdbeMemCast(Mem * pMem, u8 aff)
>> {
>> if (pMem->flags & MEM_Null)
>> - return;
>> - switch (aff) {
>> - case AFFINITY_BLOB:{ /* Really a cast to BLOB */
>> - if ((pMem->flags & MEM_Blob) == 0) {
>> - sqlite3ValueApplyAffinity(pMem, AFFINITY_TEXT);
>> - assert(pMem->flags & MEM_Str
>> - || pMem->db->mallocFailed);
>> - if (pMem->flags & MEM_Str)
>> - MemSetTypeFlag(pMem, MEM_Blob);
>> - } else {
>> - pMem->flags &= ~(MEM_TypeMask & ~MEM_Blob);
>> - }
>> - break;
>> - }
>> - case AFFINITY_NUMERIC:{
>> - sqlite3VdbeMemNumerify(pMem);
>> - break;
>> - }
>> - case AFFINITY_INTEGER:{
>> - sqlite3VdbeMemIntegerify(pMem);
>> - break;
>> - }
>> - case AFFINITY_REAL:{
>> - sqlite3VdbeMemRealify(pMem);
>> - break;
>> + return SQLITE_OK;
>> + if (pMem->flags & MEM_Blob && aff == AFFINITY_INTEGER)
>> + return sql_atoi64(pMem->z, &pMem->u.i, pMem->n);
>> + if (pMem->flags & MEM_Blob &&
>> + (aff == AFFINITY_REAL || aff == AFFINITY_NUMERIC)) {
>> + if (sql_atoi64(pMem->z, &pMem->u.i, pMem->n) == 0) {
>> + MemSetTypeFlag(pMem, MEM_Real);
>> + pMem->u.r = pMem->u.i;
>> + return 0;
>> }
>
> 5. I guess, you can move these two checks down to the switch into
> corresponding cases to avoid unnecessary checks. See my review fixes.
> And if you applied them, I would answer why not to fix
> sqlite3VdbeIntValue to allow cast BLOB to int as you do here out
> of sqlite3VdbeIntValue?
For the reason that sqlite3VdbeMemCast() is called only on explicit
CAST() function. If I moved cast from BLOB to INT to general
sqlite3VdbeIntValue() then all conversions from BLOB to INT
would be legal, but it is not correct at all. TEXT can be converted
to INT (however, Peter Gulutzan against even this conversion),
but BLOB shouldn’t (since most DBs store it as a real binary string and
no implicit conversions take place).
> I mean to delete this 'if' above:
>
> if (pMem->flags & MEM_Blob && aff == AFFINITY_INTEGER)
>
> and allow 'case AFFINITY_INTEGER:' do its work below.
>
> I did not move this case:
>
> if ((pMem->flags & MEM_Blob) != 0 &&
> (aff == AFFINITY_REAL || aff == AFFINITY_NUMERIC)) {
>
> Please, do it by yourself.
If I moved these checks, we would get code duplication (the same chunks
under case NUMERIC and case REAL). Do you really want it? Again, I can’t
move these conversion into under-the-hood functions like sqlite3VdbeMemNumerify().
I wouldn’t bother much about code clearness in this function in particular and in
patch in general, since in the next release (2.2) we are going to move all
(or at least most of) these runtime checks to query compilation stage.
I assume that the main goal of this patch-set is likely to define user-visible
types behaviour e.g. which conversions are allowed and which are not etc.
>> diff --git a/test/sql-tap/boundary2.test.lua b/test/sql-tap/boundary2.test.lua
>> index 3eaef75dc..be4b8750d 100755
>> --- a/test/sql-tap/boundary2.test.lua
>> +++ b/test/sql-tap/boundary2.test.lua
>> @@ -1,6 +1,6 @@
>> #!/usr/bin/env tarantool
>> test = require("sqltester")
>> -test:plan(3021)
>> +test:plan(2965)
>> --!./tcltestrunner.lua
>> -- 2008 December 11
>> @@ -7462,6 +7462,7 @@ test:do_execsql_test(
>> "SELECT a FROM t1 WHERE r > 9.22337303685477580800e+18 ORDER BY a DESC",
>> {})
>> +if false then
>
> 6. I thought you have removed all these 'if false'. Please, do it.
I did it in boundary3 test and forgot about this one. Fixed:
diff --git a/test/sql-tap/boundary2.test.lua b/test/sql-tap/boundary2.test.lua
index be4b8750d..3eaef75dc 100755
--- a/test/sql-tap/boundary2.test.lua
+++ b/test/sql-tap/boundary2.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(2965)
+test:plan(3021)
--!./tcltestrunner.lua
-- 2008 December 11
@@ -7462,7 +7462,6 @@ test:do_execsql_test(
"SELECT a FROM t1 WHERE r > 9.22337303685477580800e+18 ORDER BY a DESC",
{})
-if false then
test:do_execsql_test(
"boundary2-2.65.gt.3",
"SELECT a FROM t1 WHERE r > 9.22337303685477580800e+18 ORDER BY r",
@@ -7472,7 +7471,6 @@ test:do_execsql_test(
"boundary2-2.65.gt.4",
"SELECT a FROM t1 WHERE r > 9.22337303685477580800e+18 ORDER BY r DESC",
{})
-end
test:do_execsql_test(
"boundary2-2.65.gt.5",
@@ -7489,7 +7487,6 @@ test:do_execsql_test(
"SELECT a FROM t1 WHERE r >= 9.22337303685477580800e+18 ORDER BY a DESC",
{})
-if false then
test:do_execsql_test(
"boundary2-2.65.ge.3",
"SELECT a FROM t1 WHERE r >= 9.22337303685477580800e+18 ORDER BY r",
@@ -7499,7 +7496,6 @@ test:do_execsql_test(
"boundary2-2.65.ge.4",
"SELECT a FROM t1 WHERE r >= 9.22337303685477580800e+18 ORDER BY r DESC",
{})
-end
test:do_execsql_test(
"boundary2-2.65.ge.5",
@@ -7516,7 +7512,6 @@ test:do_execsql_test(
"SELECT a FROM t1 WHERE r < 9.22337303685477580800e+18 ORDER BY a DESC",
{64, 63, 62, 61, 60, 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1})
-if false then
test:do_execsql_test(
"boundary2-2.65.lt.3",
"SELECT a FROM t1 WHERE r < 9.22337303685477580800e+18 ORDER BY r",
@@ -7526,7 +7521,6 @@ test:do_execsql_test(
"boundary2-2.65.lt.4",
"SELECT a FROM t1 WHERE r < 9.22337303685477580800e+18 ORDER BY r DESC",
{3, 28, 17, 45, 27, 43, 13, 26, 10, 34, 25, 56, 7, 19, 57, 35, 46, 22, 39, 36, 14, 51, 20, 40, 12, 6, 9, 24, 18, 42, 15, 62, 48, 50, 23, 16, 8, 61, 30, 49, 4, 31, 5, 41, 60, 59, 38, 33, 52, 53, 54, 32, 29, 37, 1, 11, 47, 63, 58, 44, 21, 64, 2, 55})
-end
test:do_execsql_test(
"boundary2-2.65.lt.5",
@@ -7543,7 +7537,6 @@ test:do_execsql_test(
"SELECT a FROM t1 WHERE r <= 9.22337303685477580800e+18 ORDER BY a DESC",
{64, 63, 62, 61, 60, 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1})
-if false then
test:do_execsql_test(
"boundary2-2.65.le.3",
"SELECT a FROM t1 WHERE r <= 9.22337303685477580800e+18 ORDER BY r",
@@ -7553,7 +7546,6 @@ test:do_execsql_test(
"boundary2-2.65.le.4",
"SELECT a FROM t1 WHERE r <= 9.22337303685477580800e+18 ORDER BY r DESC",
{3, 28, 17, 45, 27, 43, 13, 26, 10, 34, 25, 56, 7, 19, 57, 35, 46, 22, 39, 36, 14, 51, 20, 40, 12, 6, 9, 24, 18, 42, 15, 62, 48, 50, 23, 16, 8, 61, 30, 49, 4, 31, 5, 41, 60, 59, 38, 33, 52, 53, 54, 32, 29, 37, 1, 11, 47, 63, 58, 44, 21, 64, 2, 55})
-end
test:do_execsql_test(
"boundary2-2.65.le.5",
@@ -7570,7 +7562,6 @@ test:do_execsql_test(
"SELECT a FROM t1 WHERE r > -9.22337303685477580800e+18 ORDER BY a DESC",
{64, 63, 62, 61, 60, 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1})
-if false then
test:do_execsql_test(
"boundary2-2.66.gt.3",
"SELECT a FROM t1 WHERE r > -9.22337303685477580800e+18 ORDER BY r",
@@ -7580,7 +7571,6 @@ test:do_execsql_test(
"boundary2-2.66.gt.4",
"SELECT a FROM t1 WHERE r > -9.22337303685477580800e+18 ORDER BY r DESC",
{3, 28, 17, 45, 27, 43, 13, 26, 10, 34, 25, 56, 7, 19, 57, 35, 46, 22, 39, 36, 14, 51, 20, 40, 12, 6, 9, 24, 18, 42, 15, 62, 48, 50, 23, 16, 8, 61, 30, 49, 4, 31, 5, 41, 60, 59, 38, 33, 52, 53, 54, 32, 29, 37, 1, 11, 47, 63, 58, 44, 21, 64, 2, 55})
-end
test:do_execsql_test(
"boundary2-2.66.gt.5",
@@ -7597,7 +7587,6 @@ test:do_execsql_test(
"SELECT a FROM t1 WHERE r >= -9.22337303685477580800e+18 ORDER BY a DESC",
{64, 63, 62, 61, 60, 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1})
-if false then
test:do_execsql_test(
"boundary2-2.66.ge.3",
"SELECT a FROM t1 WHERE r >= -9.22337303685477580800e+18 ORDER BY r",
@@ -7607,7 +7596,6 @@ test:do_execsql_test(
"boundary2-2.66.ge.4",
"SELECT a FROM t1 WHERE r >= -9.22337303685477580800e+18 ORDER BY r DESC",
{3, 28, 17, 45, 27, 43, 13, 26, 10, 34, 25, 56, 7, 19, 57, 35, 46, 22, 39, 36, 14, 51, 20, 40, 12, 6, 9, 24, 18, 42, 15, 62, 48, 50, 23, 16, 8, 61, 30, 49, 4, 31, 5, 41, 60, 59, 38, 33, 52, 53, 54, 32, 29, 37, 1, 11, 47, 63, 58, 44, 21, 64, 2, 55})
-end
test:do_execsql_test(
"boundary2-2.66.ge.5",
@@ -7624,7 +7612,6 @@ test:do_execsql_test(
"SELECT a FROM t1 WHERE r < -9.22337303685477580800e+18 ORDER BY a DESC",
{})
-if false then
test:do_execsql_test(
"boundary2-2.66.lt.3",
"SELECT a FROM t1 WHERE r < -9.22337303685477580800e+18 ORDER BY r",
@@ -7634,7 +7621,6 @@ test:do_execsql_test(
"boundary2-2.66.lt.4",
"SELECT a FROM t1 WHERE r < -9.22337303685477580800e+18 ORDER BY r DESC",
{})
-end
test:do_execsql_test(
"boundary2-2.66.lt.5",
@@ -7651,7 +7637,6 @@ test:do_execsql_test(
"SELECT a FROM t1 WHERE r <= -9.22337303685477580800e+18 ORDER BY a DESC",
{})
-if false then
test:do_execsql_test(
"boundary2-2.66.le.3",
"SELECT a FROM t1 WHERE r <= -9.22337303685477580800e+18 ORDER BY r",
@@ -7661,7 +7646,6 @@ test:do_execsql_test(
"boundary2-2.66.le.4",
"SELECT a FROM t1 WHERE r <= -9.22337303685477580800e+18 ORDER BY r DESC",
{})
-end
test:do_execsql_test(
"boundary2-2.66.le.5",
@@ -15027,7 +15011,6 @@ test:do_execsql_test(
"SELECT a FROM t1 WHERE r <= 3 ORDER BY x",
{59, 60, 41, 5, 55, 2, 64, 21, 44, 58, 63, 47, 11, 1, 37, 29, 32, 54, 53, 52, 33, 38})
-if false then
test:do_execsql_test(
"boundary2-4.65.gt.1",
"SELECT a FROM t1 WHERE r > 9.22337303685477580800e+18 ORDER BY a",
@@ -15227,6 +15210,5 @@ test:do_execsql_test(
"boundary2-4.66.le.5",
"SELECT a FROM t1 WHERE r <= -9.22337303685477580800e+18 ORDER BY x",
{})
-end
>
>> test:do_execsql_test(
>> "boundary2-2.65.gt.3",
>> "SELECT a FROM t1 WHERE r > 9.22337303685477580800e+18 ORDER BY r",
>> diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua
>> index eb4f43a90..e5a2d7a5a 100755
>> --- a/test/sql-tap/collation.test.lua
>> +++ b/test/sql-tap/collation.test.lua
>> @@ -250,8 +250,8 @@ local like_testcases =
>> test:do_catchsql_set_test(like_testcases, prefix)
>> test:do_catchsql_test(
>> - "collation-2.5.0",
>> - 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))',
>> - {1, "Collation 'FOO' does not exist"})
>> + "collation-2.5.0",
>> + 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))',
>> + {1, "Collation 'FOO' does not exist"})
>
> 7. Nothing has changed. Stray diff.
Ok (it was incorrectly formatted btw):
diff --git a/test/sql-tap/collation.test.lua b/test/sql-tap/collation.test.lua
index e5a2d7a5a..eb4f43a90 100755
--- a/test/sql-tap/collation.test.lua
+++ b/test/sql-tap/collation.test.lua
@@ -250,8 +250,8 @@ local like_testcases =
test:do_catchsql_set_test(like_testcases, prefix)
test:do_catchsql_test(
- "collation-2.5.0",
- 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))',
- {1, "Collation 'FOO' does not exist"})
+ "collation-2.5.0",
+ 'CREATE TABLE test3 (a int, b int, c int, PRIMARY KEY (a, a COLLATE foo, b, c))',
+ {1, "Collation 'FOO' does not exist"})
>
>> test:finish_test()
>> diff --git a/test/sql-tap/like3.test.lua b/test/sql-tap/like3.test.lua
>> index ea6824ba7..a683df2d6 100755
>> --- a/test/sql-tap/like3.test.lua
>> +++ b/test/sql-tap/like3.test.lua
>> @@ -75,9 +75,9 @@ test:do_execsql_test(
>> CREATE INDEX t2ba ON t2(b,a);
>> SELECT a, b FROM t2 WHERE b GLOB 'ab*' ORDER BY +a;
>> ]], {
>> - -- <like3-2.0>
>> - 1, "abc", 4, "abc"
>> - -- </like3-2.0>
>> + -- <like3-2.0>
>> + 1, "abc", 4, "abc"
>> + -- </like3-2.0>
>
> 8. The same, and below.
diff --git a/test/sql-tap/like3.test.lua b/test/sql-tap/like3.test.lua
index 505d2fabb..f9adb31bd 100755
--- a/test/sql-tap/like3.test.lua
+++ b/test/sql-tap/like3.test.lua
@@ -35,14 +35,14 @@ test:plan(7)
test:execsql([[
--PRAGMA encoding='UTF8';
- CREATE TABLE t1(a PRIMARY KEY,b TEXT COLLATE "unicode_ci");
+ CREATE TABLE t1(a INT PRIMARY KEY,b TEXT COLLATE "unicode_ci");
INSERT INTO t1(a,b)
VALUES(1,'abc'),
(2,'ABX'),
(3,'BCD'),
- (4,x'616263'),
- (5,x'414258'),
- (6,x'424344');
+ (4, char(0x61, 0x62, 0x63)),
+ (5, char(0x41, 0x42, 0x58)),
+ (6, char(0x42, 0x43, 0x44));
CREATE INDEX t1ba ON t1(b,a);
]])
@@ -70,7 +70,7 @@ test:do_execsql_test(
test:do_execsql_test(
"like3-2.0",
[[
- CREATE TABLE t2(a PRIMARY KEY, b TEXT);
+ CREATE TABLE t2(a INT PRIMARY KEY, b TEXT);
INSERT INTO t2 SELECT a, b FROM t1;
CREATE INDEX t2ba ON t2(b,a);
SELECT a, b FROM t2 WHERE b GLOB 'ab*' ORDER BY +a;
@@ -93,46 +93,46 @@ test:do_execsql_test(
test:do_execsql_test(
"like3-2.2",
[[
- SELECT a, b FROM t2 WHERE b>=x'6162' AND b GLOB 'ab*'
+ SELECT a, b FROM t2 WHERE b>='ab' AND b GLOB 'ab*'
]], {
-- <like3-2.2>
- 4, "abc"
+ 1, "abc", 4, "abc"
-- </like3-2.2>
})
test:do_execsql_test(
"like3-2.3",
[[
- SELECT a, b FROM t2 WHERE +b>=x'6162' AND +b GLOB 'ab*'
+ SELECT a, b FROM t2 WHERE +b>='ab' AND +b GLOB 'ab*'
]], {
-- <like3-2.3>
- 4, "abc"
+ 1, "abc", 4, "abc"
-- </like3-2.3>
})
test:do_execsql_test(
"like3-2.4",
[[
- SELECT a, b FROM t2 WHERE b GLOB 'ab*' AND b>=x'6162'
+ SELECT a, b FROM t2 WHERE b GLOB 'ab*' AND b>='ab'
]], {
-- <like3-2.4>
- 4, "abc"
+ 1, "abc", 4, "abc"
-- </like3-2.4>
})
test:do_execsql_test(
"like3-2.5",
[[
- SELECT a, b FROM t2 WHERE +b GLOB 'ab*' AND +b>=x'6162'
+ SELECT a, b FROM t2 WHERE +b GLOB 'ab*' AND +b>='ab'
]], {
-- <like3-2.5>
- 4, "abc"
+ 1, "abc", 4, "abc"
-- </like3-2.5>
})
+
test:execsql([[
CREATE TABLE t3(x TEXT PRIMARY KEY COLLATE "unicode_ci");
INSERT INTO t3(x) VALUES('aaa'),('abc'),('abd'),('abe'),('acz');
- INSERT INTO t3(x) SELECT CAST(x AS blob) FROM t3;
]])
>
>> })
>> test:do_execsql_test(
>> @@ -85,54 +85,55 @@ test:do_execsql_test(
>> [[
>> SELECT a, b FROM t2 WHERE +b GLOB 'ab*' ORDER BY +a;
>> ]], {
>> - -- <like3-2.1>
>> - 1, "abc", 4, "abc"
>> - -- </like3-2.1>
>> - })
>> + -- <like3-2.1>
>> + 1, "abc", 4, "abc"
>> + -- </like3-2.1>
>> + })
>> test:execsql([[
>> CREATE TABLE t3(x TEXT PRIMARY KEY COLLATE "unicode_ci");
>> INSERT INTO t3(x) VALUES('aaa'),('abc'),('abd'),('abe'),('acz');
>> - INSERT INTO t3(x) SELECT CAST(x AS blob) FROM t3;
>> +-- INSERT INTO t3(x) SELECT CAST(x AS blob) FROM t3;
>
> 9. Delete or uncomment, please.
Done, see above.
>> ]])
>> -- MUST_WORK #1476 collate nocase
>> diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
>> index f917e5a51..6750cefef 100755
>> --- a/test/sql-tap/numcast.test.lua
>> +++ b/test/sql-tap/numcast.test.lua
>> @@ -1,6 +1,6 @@
>> #!/usr/bin/env tarantool
>> test = require("sqltester")
>> -test:plan(17)
>> +test:plan(9)
>> --!./tcltestrunner.lua
>> -- 2013 March 20
>> @@ -38,11 +38,8 @@ for _, enc in ipairs({"utf8"}) do
>> {"1", "12345.0", 12345.0, 12345},
>> {"2", "12345.0e0", 12345.0, 12345},
>> {"3", "-12345.0e0", -12345.0, -12345},
>> - {"4", "-12345.25", -12345.25, -12345},
>> +-- {"4", "-12345.25", -12345.25, -12345},
>
> 10. —
Uncommented.
>
>> {"5", "-12345.0", -12345.0, -12345},
>> - {"6", "'876xyz'", 876.0, 876},
>> - {"7", "'456ķ89'", 456.0, 456},
>> - {"8", "'Ġ 321.5'", 0.0, 0},
>> }
>> for _, val in ipairs(data) do
>> local idx = val[1]
> Review fixes:
Applied as obvious.
next prev parent reply other threads:[~2018-10-23 23:28 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-17 20:32 [tarantool-patches] [PATCH 0/6] Introduce strict typing for SQL Nikita Pettik
2018-09-17 20:32 ` [tarantool-patches] [PATCH 1/6] sql: split conflict action and affinity for Expr Nikita Pettik
2018-09-19 2:16 ` [tarantool-patches] " Konstantin Osipov
2018-09-27 20:24 ` Vladislav Shpilevoy
2018-10-12 11:18 ` n.pettik
2018-09-17 20:32 ` [tarantool-patches] [PATCH 2/6] sql: annotate SQL functions with return type Nikita Pettik
2018-09-27 20:23 ` [tarantool-patches] " Vladislav Shpilevoy
2018-10-12 11:18 ` n.pettik
2018-09-17 20:32 ` [tarantool-patches] [PATCH 3/6] sql: pass true types of columns to Tarantool Nikita Pettik
2018-09-19 2:23 ` [tarantool-patches] " Konstantin Osipov
2018-10-12 11:19 ` n.pettik
2018-09-27 20:23 ` Vladislav Shpilevoy
2018-10-12 11:18 ` n.pettik
2018-10-17 21:45 ` Vladislav Shpilevoy
2018-10-23 23:28 ` n.pettik
2018-10-29 21:32 ` Vladislav Shpilevoy
2018-11-02 2:36 ` n.pettik
2018-09-17 20:32 ` [tarantool-patches] [PATCH 4/6] sql: enforce implicit type conversions Nikita Pettik
2018-09-19 2:25 ` [tarantool-patches] " Konstantin Osipov
2018-09-27 20:24 ` Vladislav Shpilevoy
2018-10-12 11:19 ` n.pettik
2018-10-17 21:45 ` Vladislav Shpilevoy
2018-10-23 23:28 ` n.pettik [this message]
2018-10-29 21:32 ` Vladislav Shpilevoy
2018-11-02 2:36 ` n.pettik
2018-11-02 11:15 ` Vladislav Shpilevoy
2018-11-02 13:26 ` n.pettik
2018-09-17 20:32 ` [tarantool-patches] [PATCH 5/6] sql: return result-set type via IProto Nikita Pettik
2018-09-19 2:26 ` [tarantool-patches] " Konstantin Osipov
2018-09-27 20:24 ` Vladislav Shpilevoy
2018-10-12 11:19 ` n.pettik
2018-10-17 21:45 ` Vladislav Shpilevoy
2018-10-23 23:28 ` n.pettik
2018-09-17 20:32 ` [tarantool-patches] [PATCH 6/6] sql: discard numeric conversion by unary plus Nikita Pettik
2018-09-27 20:24 ` [tarantool-patches] " Vladislav Shpilevoy
2018-10-12 11:19 ` n.pettik
2018-09-27 20:24 ` [tarantool-patches] Re: [PATCH 0/6] Introduce strict typing for SQL Vladislav Shpilevoy
2018-10-12 11:18 ` n.pettik
2018-11-03 2:41 ` 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=E2586704-DF9B-4D7D-9E01-F1D20B8A6B6F@tarantool.org \
--to=korablev@tarantool.org \
--cc=tarantool-patches@freelists.org \
--cc=v.shpilevoy@tarantool.org \
--subject='[tarantool-patches] Re: [PATCH 4/6] sql: enforce implicit type conversions' \
/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