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 arenot necessary, IMHO. I can not imagine how many hundreds ofyears a one should insert into one ephemeral table toreach 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.