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.
next prev parent 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