Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 2/4] sql: properly show values in type mismatch error
Date: Mon, 12 Jul 2021 11:56:08 +0300
Message-ID: <20210712085608.GB127980@tarantool.org> (raw)
In-Reply-To: <281c365c-afe9-0c36-526b-f9edb27452d2@tarantool.org>

Hi! Thank you for the review! My answer below.

On Thu, Jul 08, 2021 at 12:09:57AM +0200, Vladislav Shpilevoy wrote:
> Thanks for the patch!
> 
> > diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> > index 630f1a135..728d3c9a7 100644
> > --- a/src/box/sql/mem.c
> > +++ b/src/box/sql/mem.c
> > @@ -80,9 +80,11 @@ mem_str(const struct Mem *mem)
> >                 if (mem->n > STR_VALUE_MAX_LEN) {
> >                         memcpy(buf, mem->z, STR_VALUE_MAX_LEN);
> >                         buf[STR_VALUE_MAX_LEN] = '\0';
> > -                       return tt_sprintf("%s...", buf);
> > +                       return tt_sprintf("'%s...", buf);
> 
> Why didn't you put a second ' after ...? The error message
> does not end here. There are more words after this string usually.
> The same below.
> 
I belive that "'" is actually part of value. This way we will get inconsistent
value in case only part of it was printed. This is similar for what we do with
array - in case it is too long we will not close it using "]".

Same for varbinary.

> > @@ -102,7 +115,8 @@ mem_str(const struct Mem *mem)
> >                 return tt_sprintf("%s...", buf);
> >         }
> >         case MEM_TYPE_UUID:
> > -               return tt_uuid_str(&mem->u.uuid);
> > +               tt_uuid_to_string(&mem->u.uuid, buf);
> > +               return tt_sprintf("'%s'", buf);
> >         case MEM_TYPE_BOOL:
> >                 return mem->u.b ? "TRUE" : "FALSE";
> >         default:

  reply	other threads:[~2021-07-12  8:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-05 15:27 [Tarantool-patches] [PATCH v2 0/4] sql: fix description of " Mergen Imeev via Tarantool-patches
2021-07-05 15:27 ` [Tarantool-patches] [PATCH v2 1/4] sql: truncate values in " Mergen Imeev via Tarantool-patches
2021-07-07 22:09   ` Vladislav Shpilevoy via Tarantool-patches
2021-07-12  8:50     ` Mergen Imeev via Tarantool-patches
2021-07-05 15:27 ` [Tarantool-patches] [PATCH v2 2/4] sql: properly show " Mergen Imeev via Tarantool-patches
2021-07-07 22:09   ` Vladislav Shpilevoy via Tarantool-patches
2021-07-12  8:56     ` Mergen Imeev via Tarantool-patches [this message]
2021-07-05 15:27 ` [Tarantool-patches] [PATCH v2 3/4] sql: use proper type names in error descriptions Mergen Imeev via Tarantool-patches
2021-07-05 15:27 ` [Tarantool-patches] [PATCH v2 4/4] sql: make type mismatch error more informative Mergen Imeev via Tarantool-patches
2021-07-07 22:10   ` Vladislav Shpilevoy via Tarantool-patches
2021-07-12  8:58     ` Mergen Imeev via Tarantool-patches
2021-07-12 21:38 ` [Tarantool-patches] [PATCH v2 0/4] sql: fix description of type mismatch error Vladislav Shpilevoy via Tarantool-patches
2021-07-13  7:03 [Tarantool-patches] [PATCH v2 0/4] sql: fix description of type mismatch errors Mergen Imeev via Tarantool-patches
2021-07-13  7:03 ` [Tarantool-patches] [PATCH v2 2/4] sql: properly show values in type mismatch error Mergen Imeev via Tarantool-patches
2021-07-13  8:51   ` Timur Safin via Tarantool-patches

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=20210712085608.GB127980@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    /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

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git