[tarantool-patches] Re: [PATCH 5/6] sql: introduce extended range for INTEGER type

["n.pettik" <korablev@tarantool.org>]

>> 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);