From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp15.mail.ru (smtp15.mail.ru [94.100.176.133])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 3E48E42EF5C
 for <tarantool-patches@dev.tarantool.org>;
 Sun, 14 Jun 2020 20:03:39 +0300 (MSK)
References: <cover.1591878044.git.imeevma@gmail.com>
 <338034a2577e902a1295db12650fee26a8bcdbf2.1591878044.git.imeevma@gmail.com>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <71ed2250-9f9a-040c-debf-064d6bb4e82a@tarantool.org>
Date: Sun, 14 Jun 2020 19:03:36 +0200
MIME-Version: 1.0
In-Reply-To: <338034a2577e902a1295db12650fee26a8bcdbf2.1591878044.git.imeevma@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH v2 7/7] sql: use type instead of
 value in type mismatch error
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: imeevma@tarantool.org, tsafin@tarantool.org, tarantool-patches@dev.tarantool.org

Thanks for the patch!

> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index e22ed98cf..cf0ae8a06 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -1748,12 +1748,12 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
>  	} else {
>  		if (sqlVdbeRealValue(pIn1, &rA) != 0) {
>  			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> -				 sql_value_to_diag_str(pIn1), "numeric");
> +				 mem_type_to_str(pIn1), "numeric");

src/box/sql/func.c still contains lots of sql_value_to_diag_str() calls.
I think, sql_value_to_diag_str() should be removed completely.

It is not only inconsistent with other error messages, but is also unsafe,
because that errors may end up in user logs, and may contain sensitive
data such as password hashes.

Please, remove it in this commit, and then send the whole patchset to
Nikita for a second review. The patchset LGTM except the comment above.