[tarantool-patches] Re: [PATCH 1/2] space: add method to fetch next rowid

["n.pettik" <korablev@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 3313 bytes --]



> On 9 Nov 2018, at 12:25, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Hi! Thanks for the patch! I understand, that Vova said
> that it should not be pushed, but Kirill asked me, on
> the contrary, to review it. So I do.

Vladimir didn’t suggest better solutions except for complete reworking
them. Now it is definitely bug which leads to wrong results of
SELECT queries (which is a terrible thing taking into account the fact that
SQL is supposed to be used mostly for DQL). So lets take this patch as
a workaround and rework ephemeral tables when we will have enough time
and resources (surely if Kirill and Vladimir don’t mind).

With this bug it seems to be unacceptable to release beta version.

> On 29/10/2018 22:02, Nikita Pettik wrote:
>> Ephemeral space are extensively used in SQL to store intermediate
>> results of query processing. To keep things simple, they feature only
>> one unique index (primary) which covers all fields. However, ephemeral
>> space can be used to store non-unique entries. In this case, one
>> additional field added to the end if stored data:
>> [field1, ... fieldn, rowid]
>> Note that it can't be added to the beginning of tuple since data in
>> ephemeral space may be kept as sorted. Previously, to generate proper
>> rowid index_max() was used. However, it is obviously wrong way to do it.
>> Hence, lets add simple integer counter to memtx space (ephemeral spaces
>> are valid only for memtx engine) and introduce method in vtab to fetch
>> next rowid value.
>> Needed for #3297
>> ---
>>  src/box/blackhole.c    |  1 +
>>  src/box/errcode.h      |  2 ++
>>  src/box/memtx_space.c  | 17 +++++++++++++++++
>>  src/box/memtx_space.h  |  7 +++++++
>>  src/box/space.c        |  9 +++++++++
>>  src/box/space.h        |  3 +++
>>  src/box/sysview.c      |  1 +
>>  src/box/vinyl.c        |  1 +
>>  src/errinj.h           |  1 +
>>  test/box/errinj.result |  2 ++
>>  test/box/misc.result   |  1 +
>>  11 files changed, 45 insertions(+)
>> index 04f4f34ee..fab8b6617 100644
>> --- a/src/box/errcode.h
>> +++ b/src/box/errcode.h
>> @@ -223,6 +223,8 @@ struct errcode_record {
>>  	/*168 */_(ER_DROP_FK_CONSTRAINT,	"Failed to drop foreign key constraint '%s': %s") \
>>  	/*169 */_(ER_NO_SUCH_CONSTRAINT,	"Constraint %s does not exist") \
>>  	/*170 */_(ER_CONSTRAINT_EXISTS,		"Constraint %s already exists") \
>> +	/*171 */_(ER_ROWID_OVERFLOW,		"Rowid is overflowed: too many entries in ephemeral space") \
>> +
> 
> This error message as well as check on uint64_max are
> not necessary, IMHO. I can not imagine how many hundreds of
> years a one should insert into one ephemeral table to
> reach this limit.

It is true that 2^64 is likely to be quite huge number of tuples,
but for instance JOIN uses nested-loop algorithm, so it requires
n^2 memory for ephemeral table to comprise results.
In this regard, to reach the limit we need 4-way join where each
table contains 2^16 entries, which in turn doesn’t seem to be giant.

*It is only thoughts tho, I haven’t tested it since I suppose very likely
 my pc would simply get stuck.*

I wanted to create long test as the easiest solution, but Alexander warned
me that Travis may not survive such test due to lack of memory.



[-- Attachment #2: Type: text/html, Size: 11822 bytes --]