> On 12 Nov 2018, at 02:22, Vladislav Shpilevoy wrote: > On 12/11/2018 02:16, n.pettik wrote: >>> On 9 Nov 2018, at 12:25, Vladislav Shpilevoy > 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. > > I do not mind, if you drop my fixes. It is just nitpicking. The > patchset is generally ok already. Actually, I don’t mind your fixes as well, so I am going to apply them. Also, I’ve rebased patch-set on fresh 2.1.