I actually realized this is a terrible abstraction the day I implemented
it and have since been planning to remove it.
The issue is with aliasing and potential corruption in a multi-threaded
environment. Even though we have never used it in such a context (aka
triggering UB) but we can potentially make an *accidental* use of it;
don't take it lightly: it's like modifying data while someone else is
reading it. That's not bad, that's a nightmare.
Discovered another security bug which could potentially be dangerous:
the low level impl of the index used a comparator where eager evaluation
of an expression led to an erroneous return.
This commit makes an important security fix that caused non-matching
of field data which could have had terrible impacts.
Second, it adds tests for insert and delete.
This was a bad mistake that we made with the actions API, and we won't
make the same mistake with BlueQL again. PES has been a terrible idea
all along, and it was a leaky abstraction.
Instead, we'll be using a more robust framework for addressing entities.
This is one huge commit that fixes a bunch of issues we've had with
the schema and surrounding impls. Here are some of them
- The type metadata syn parse was incredibly broken. More than
one trailing comma or other punctuation following a trailing symbol
(as in a comma) is illegal
- The `*_full` set of methods were repetitive and annoying. That has
been fixed (not the most elegant solution, but it works and fits
well)
- As a result of the first issue, fuzz targets kept blowing up,
rightfully. This has been fixed too. The fuzz needs to be more
deterministic covering all permutations, and this is something I'll
fix up ahead.
Finally, I can now happily say that the QL impl is rock solid. There is
another possible bug that I'm tracking down, but that will be fixed.
This hasn't been an easy task, but I'm finally very happy that we've
gotten rid of all usage instances of `RawSlice` within `engine::ql`.
This is a great feat in terms of codebase cleanup, removing unnecessary
usage of `unsafe` code.
However, this had made lifetimes slightly more complex; for example, we
can no longer directly return from methods that use `&[Token]` since all
returned data is now bound to the same lifetime.
Either way, with some care such issues can be avoided.
Along with this, here's a summary of other changes:
- `RawSlice` now uses `NonNull` to help the compiler with size opts
- The lexer now allows case insensitive keyword usage (I still DO NOT
particularly approve of it, because enforcing is important)
Summary of changes:
- Remaining query parse methods that directly used the `Compiler` were
modified to use token streams instead
- `Entity::parse_from_tokens` to fix assignment to counter instead of
increments
- Allow entity in `drop model`
- `alter model` and `alter space` directly handle space/model names
instead of depending on passing value via args
- Tests added for `drop` and `inspect`
Summary of changes:
- The `drop` queries now use the `DropItem` for drop definitions
- `create model` is now entirely handled by the function in `schema`
- `create space` is now entirely handled by the function in `schema`
- Tests were added for drop (they were never present before)
Per our language specification, unsafe literals are by their inherent
name literals, hence keep them under this variant. Also, it simplifies
a bunch of implementations. Although I'm sure we can do something for
performance to simply provide an error case when we encounter a
disallowed literal.
* Re-implement network stack
This commit does a complete overhaul to the networking subsystem, and this has
been pending for a long time. For whatever reason, when some generic
flexibility was aimed for, we went ahead with the "expensive emulation" of
async traits which has resulted in unnecessary complication with type arguments
(lifetimes, function signatures looking insane et cetera) besides the usual
downsides.
Also, the termination signaling mechanism was slightly excessive because it was
built with the idea of supporting PUB/SUB by default.
Last but not the least, the authentication system used dynamism which had
resulted in unwanted complication to type arguments for functions that accepted
the auth handle. Now, none of those complications exist and we have an almost
glorious life (for now atleast).
* Fix simple query header for Skyhash-1.0
This ensures that we always have an aligned pointer, even
when the slice is zero sized. This is required for enum
optimizations (as noted in from_raw_parts).
* Switch to using `SharedSlice`
Although I'm a huge fan of case-based optimizations, we don't exactly need the
sophistication that `Bytes` gives us. Instead, we switch to a traditional ref
counted structure, which allocates on creation and deallocates when it is the
only owner (bytes also does this albeit using dynamic dispath via a vft).
The same function path is utilized for every operation on the structure. The
difference between heap-allocating the shared state and having it as fields
on the base structure is a micro-optimization that I'm yet to substantiate
with performance gains.
* Add tests for `SharedSlice`
* Fix erroneous removal of auth table during tree cleanup
When it comes to the handling of system tables, the current storage engine has
a little "funk." It is because the limited flexibility of the format prevents
us from directly storing system tables which have a different structure than
ordinary user created tables; this is why when the cleanup is run, previously,
the dummy keyspace created for the PRELOAD was scanned and since it has nothing
any "alien" file in the `ks/system` directory was purged. This commit fixes
that and ensures that we do not clean up the system directory.
This is also another reason why a new storage engine coupled with a new memory
engine are being developed. These shortcomings will be addressed with the new
engines.
* Add test case for issue #276
Add Changelog entry
I initially planned to implement a scanner-less parser, but however, the
complexity was unnecessary and the performance improvements were marginal.
With increasing language complexity, such a pipeline would become even more
complex and hence, I re-implemented the parser with a traditional "proven"
lexer-parser pipeline. More performance improvements are possible, and I
will add them ahead.
In an earlier commit we marked `as_slice` as safe to call, stating
that only construction is unsafe. However, that is incorrect. The
ctor of `UnsafeSlice` does nothing unsafe, unless we make as_slice
safe. However, since the type is not bounded to any lifetime,
making `as_slice` safe to call assumes a very "rarely true"
safety contract: that the pointers are valid throughout the
execution of the program, id est they are static.
However, that assumption is entirely incorrect for our use case,
hence I'm marking this as `unsafe` again.
Sayan Nandan