[tarantool-patches] Re: [PATCH 5/6] sql: introduce extended range for INTEGER type
n.pettik
korablev at tarantool.org
Mon Jul 1 17:21:44 MSK 2019
>> diff --git a/src/box/errcode.h b/src/box/errcode.h
>> index f4aba2f54..417029adb 100644
>> --- a/src/box/errcode.h
>> +++ b/src/box/errcode.h
>> @@ -242,7 +242,7 @@ struct errcode_record {
>> /*187 */_(ER_SQL_ANALYZE_ARGUMENT, "ANALYZE statement argument %s is not a base table") \
>> /*188 */_(ER_SQL_COLUMN_COUNT_MAX, "Failed to create space '%s': space column count %d exceeds the limit (%d)") \
>> /*189 */_(ER_HEX_LITERAL_MAX, "Hex literal %s%s length %d exceeds the supported limit (%d)") \
>> - /*190 */_(ER_INT_LITERAL_MAX, "Integer literal %s exceeds the supported range %lld - %lld") \
>> + /*190 */_(ER_INT_LITERAL_MAX, "Integer literal %s exceeds the supported range %lld - %llu") \
>
> That error is used only once and with the constants. Please, inline them, and
> make that error argument-less. Also, alongside I propose you to write the range in
> [start, end] instead of 'start - end'. The latter looks like a minus.
Ok, I don’t mind:
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 65fb1a011..601cb9670 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -242,7 +242,7 @@ struct errcode_record {
/*187 */_(ER_SQL_ANALYZE_ARGUMENT, "ANALYZE statement argument %s is not a base table") \
/*188 */_(ER_SQL_COLUMN_COUNT_MAX, "Failed to create space '%s': space column count %d exceeds the limit (%d)") \
/*189 */_(ER_HEX_LITERAL_MAX, "Hex literal %s%s length %d exceeds the supported limit (%d)") \
- /*190 */_(ER_INT_LITERAL_MAX, "Integer literal %s%s exceeds the supported range %lld - %llu") \
+ /*190 */_(ER_INT_LITERAL_MAX, "Integer literal %s%s exceeds the supported range [-9223372036854775808, 18446744073709551615]") \
/*191 */_(ER_SQL_PARSER_LIMIT, "%s %d exceeds the limit (%d)") \
/*192 */_(ER_INDEX_DEF_UNSUPPORTED, "%s are prohibited in an index definition") \
/*193 */_(ER_CK_DEF_UNSUPPORTED, "%s are prohibited in a CHECK constraint definition") \
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index 4201899fd..30bd7d6ae 100755
--- a/test/sql-tap/sql-errors.test.lua
+++ b/test/sql-tap/sql-errors.test.lua
@@ -142,7 +142,7 @@ test:do_catchsql_test(
SELECT 18446744073709551616;
]], {
-- <sql-errors-1.13>
- 1,"Integer literal 18446744073709551616 exceeds the supported range -9223372036854775808 - 18446744073709551615"
+ 1,"Integer literal 18446744073709551616 exceeds the supported range [-9223372036854775808, 18446744073709551615]"
-- </sql-errors-1.13>
})
> Please, consider my review fixes here and on the branch in a separate commit:
Applied.
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 75d6579ca..93aab8501 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -918,10 +918,10 @@ emitNewSysSequenceRecord(Parse *pParse, int reg_seq_id, const char *seq_name)
>
> /* 5. Minimum */
> sqlVdbeAddOp4Dup8(v, OP_Int64, 0, first_col + 5, 0,
> - (unsigned char*)&min_usigned_long_long, P4_UINT64);
> + (unsigned char *) &min_usigned_long_long, P4_UINT64);
> /* 6. Maximum */
> sqlVdbeAddOp4Dup8(v, OP_Int64, 0, first_col + 6, 0,
> - (unsigned char*)&max_usigned_long_long, P4_UINT64);
> + (unsigned char *) &max_usigned_long_long, P4_UINT64);
> /* 7. Start */
> sqlVdbeAddOp2(v, OP_Integer, 1, first_col + 7);
More information about the Tarantool-patches
mailing list