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 v1 1/3] sql: fix truncation of DECIMAL in implicit cast
Date: Wed, 6 Oct 2021 13:50:28 +0300
Message-ID: <20211006105028.GA12212@tarantool.org> (raw)
In-Reply-To: <1e9ce07c-8615-0802-3ddf-757b5d55774f@tarantool.org>

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

On Tue, Oct 05, 2021 at 11:56:08PM +0200, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patchset!
> 
> > diff --git a/test/sql-tap/gh-6485-bugs-in-decimal.test.lua b/test/sql-tap/gh-6485-bugs-in-decimal.test.lua
> > new file mode 100755
> > index 000000000..3f63f2b76
> > --- /dev/null
> > +++ b/test/sql-tap/gh-6485-bugs-in-decimal.test.lua
> > @@ -0,0 +1,16 @@
> > +#!/usr/bin/env tarantool
> > +local test = require("sqltester")
> > +test:plan(1)
> > +
> > +-- Make sure DECIMAL is not truncated when used in an index.
> > +test:do_execsql_test(
> > +    "gh-6485-1",
> > +    [[
> > +        CREATE TABLE t(i INTEGER PRIMARY KEY);
> > +        INSERT INTO t VALUES(1), (2);
> > +        SELECT i FROM t WHERE i IN (CAST(1.1 AS DECIMAL), CAST(2.2 AS DECIMAL));
> > +        DROP TABLE t;
> > +    ]], {
> 
> Could you make the same select without creating a table? The same
> for the tests in the second commit.
> 
Both tests use indexes, so the simplest way is to use spaces. I think it is
possible to create queries that use ephemeral spaces with INTEGER indexes, but
I do not think that they will be simple.

I think I should mention that due to the work of the scheduler, the most common
searches of non-INTEGER values in an INTEGER index do not use the index, even
though the entire infrastructure is ready to use the index. This drastically
reduces the number of tests we can use to test this problem. For example, in
this case, a fullscan will be performed instead of an index search:
box.execute([[CREATE TABLE t(i INT PRIMARY KEY);]])
box.execute([[INSERT INTO t VALUES(1), (2), (3), (4);]])
box.execute([[SELECT i FROM t WHERE i = 3.0;]])

> Also you might want to make the test run only once. Now it is run
> twice with 2 engines.
Thanks, fixed:

diff --git a/test/sql-tap/engine.cfg b/test/sql-tap/engine.cfg
index a6f03307f..9d318a456 100644
--- a/test/sql-tap/engine.cfg
+++ b/test/sql-tap/engine.cfg
@@ -39,6 +39,7 @@
     "gh-6376-wrong-double-to-dec-cmp.test.lua": {},
     "gh-4077-iproto-execute-no-bind.test.lua": {},
     "gh-6375-assert-on-unsupported-ext.test.lua": {},
+    "gh-6485-bugs-in-decimal.test.lua": {},
     "*": {
         "memtx": {"engine": "memtx"},
         "vinyl": {"engine": "vinyl"}

  reply	other threads:[~2021-10-06 10:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-04 13:30 [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL Mergen Imeev via Tarantool-patches
2021-10-04 13:30 ` [Tarantool-patches] [PATCH v1 1/3] sql: fix truncation of DECIMAL in implicit cast Mergen Imeev via Tarantool-patches
2021-10-05 21:56   ` Vladislav Shpilevoy via Tarantool-patches
2021-10-06 10:50     ` Mergen Imeev via Tarantool-patches [this message]
2021-10-04 13:30 ` [Tarantool-patches] [PATCH v1 2/3] sql: fix cast of small negative DECIMAL to INTEGER Mergen Imeev via Tarantool-patches
2021-10-04 13:30 ` [Tarantool-patches] [PATCH v1 3/3] sql: do not truncate DECIMAL in LIMIT and OFFSET Mergen Imeev via Tarantool-patches
2021-10-11 21:58 ` [Tarantool-patches] [PATCH v1 0/3] Fix some bugs with DECIMAL Vladislav Shpilevoy via Tarantool-patches
2021-10-19  6:19 Mergen Imeev via Tarantool-patches
2021-10-19  6:19 ` [Tarantool-patches] [PATCH v1 1/3] sql: fix truncation of DECIMAL in implicit cast Mergen Imeev 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=20211006105028.GA12212@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