Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alex Khatskevich <avkhatskevich@tarantool.org>
To: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH 2/2] sql: add test on changes/total_changes
Date: Mon, 1 Oct 2018 19:13:09 +0300	[thread overview]
Message-ID: <c438b3d7-5753-f121-c40e-e00f417ba0da@tarantool.org> (raw)
In-Reply-To: <9C1DDEBD-C37D-4B61-96AC-832BC4DDD12A@tarantool.org>


>> I have expected the same thing.
>> However:
>> 1. sqlite works in the same way.
> It doesn’t mean anything. SQLite features a wide range of really strange things.
>
>> 2. implementing this just need adding (nChanges) to savepoint
> Seems so, but it shouldn’t be complicated.
I suppose it is not complicated.
>> 3. I suppose that we should not implement this.
> Why? I guess it is matter of discussion.
I think that it is not important, because it is relatively useless counter.
1. No one need to count exact number of changes.
2. Changing the api require solid understanding of what total_changes is
     and why we need to change this.
     It strange for me that it do not do rollback on rollback, however, 
it meant to work this
     way and it also makes sense.
> Anyway, I propose to decide what to do with TOTAL_CHANGES within this patch,
> since original issue sounds like:
>
> "Fix if easy, remove otherwise.”
>
> Adding initially wrong tests is likely to be huge mistake.
This thing works. Tests proof that.
I believe that those tests are the artefact of this issue.
If you want change something, you need to file an issue and explain,
why this change is important.

  reply	other threads:[~2018-10-01 16:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-27 12:39 [tarantool-patches] [PATCH 0/2] Add test on changes && fix typo in MakeRecord AKhatskevich
2018-09-27 12:39 ` [tarantool-patches] [PATCH 1/2] sql: fix typo in MakeRecord memory hint AKhatskevich
2018-10-01  0:12   ` [tarantool-patches] " n.pettik
2018-09-27 12:39 ` [tarantool-patches] [PATCH 2/2] sql: add test on changes/total_changes AKhatskevich
2018-09-30 23:54   ` [tarantool-patches] " n.pettik
     [not found]     ` <c5639a4e-0b51-cbaa-112c-4f47d86bbe07@tarantool.org>
2018-10-01 15:20       ` n.pettik
2018-10-01 16:13         ` Alex Khatskevich [this message]
     [not found]           ` <881CB143-0DC2-468A-90CA-0459E9EE7B15@gmail.com>
2018-10-04 10:49             ` n.pettik
     [not found]               ` <75020feb-3d2a-2707-bfdb-fd3fd2229afa@tarantool.org>
2018-10-09 12:11                 ` n.pettik
2018-10-10 14:39                   ` Alex Khatskevich
2018-10-18 14:41                     ` n.pettik

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=c438b3d7-5753-f121-c40e-e00f417ba0da@tarantool.org \
    --to=avkhatskevich@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH 2/2] sql: add test on changes/total_changes' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox