* Rewrite smoketests as python unittests
* Get all tests working and do some work on parallel unittest
* Give up on parallel unittests
* Fix CI + address comments
* Fix skip-clippy arg confusion (just use the env var)
* fix ci
* Add comments
Closes#954.
I previously avoided the evaluation of certain delta table joins as an optimization,
which relied on the fact that a tx would not include inserts and deletes for both tables,
which of course is not generally correct.
This patch includes the fully general solution.
We're seeing intermittent failures of the SDK tests in CI due to timeouts.
The timeout is meant only to fail if an expected event never happens;
the SDK tests are not interested in measuring performance at all.
This commit doubles the timeout from 30 to 60 seconds,
in the hopes that we will see fewer false failures.
As an optimization,
because we do not support strict projections in subscription queries,
selections may always assume that the op_type field is last,
when removing it.
* Incremental joins: compile once, run repeatedly.
Well, more like, compile 3 times, run repeatedly, but 3 is approximately 1.
This commit re-writes `IncrementalJoin` to be a re-usable representation
of a query plan for an incremental join,
where before it was a one-off worker for the same.
`IncrementalJoin` stores three copies of the query, compiled for `MemTable * DbTable`,
`DbTable * MemTable` and `MemTable * MemTable`.
Related to this change, `eval_incr` no longer needs an `AuthCtx`,
because we check permissions during query compilation, not execution,
and all query planning is now done ahead of time during `add_subscriber`.
As a result, many callsites, especially in tests, which used to pass an `AuthCtx`
no longer do so.
* `IncrementalJoin`: save `MemTable` headers to avoid recomputing
* Don't include `__op_type` column in incremental joins
Incremental joins never used the `__op_type` column,
as they separate deletes from inserts and eval them as separate queries.
This commit causes incremental joins to no longer include the `__op_type` column
in their `MemTable`s at all,
which simplifies the code and should remove some allocations.
* Remove `pub` on test `run_query` helper.
The module's not `pub`, so it was a pointless qualifier.
Prior to this commit, `to_mem_table_with_op_type` would call `Header::find_pos_by_name`
to locate the `__op_type` column, if it existed.
This was slow, as `Header::find_pos_by_name` is a linear scan in increasing order,
and the `__op_type` column is always either last or not present,
so `to_mem_table_with_op_type` would traverse every column in the `Header`.
This happened during every incremental evaluation for every query.
With this commit, we rely on the fact that `__op_type` is always the last column if present,
and check only the last column of the header.
* Single-table subscription queries: plan once, run repeatedly
Prior to this commit, every call to `ExecutionUnit::eval_incr`
re-invoked the query planner to convert its `eval` plan
into a plan suitable for reading from a `MemTable` of updates.
With this commit, specifically for single-table select queries,
we invoke the query planner once during `ExecutionUnit::new`,
and store the resuling `eval_incr_plan` for repeated use.
A follow-up will do the same for multi-table semijoins.
* Docstrings for `SourceSet::len` and `::is_empty`
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
* Compile select queries all the way to `QueryCode` ahead of time
---------
Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
Closes#747.
Before this change,
we would evaluate each and every query,
for each and every subscription,
on each and every row update.
If N subscriptions had a query Q in common,
it would be evaluated N different times.
With this change,
distinct queries are evaluated once,
and the results copied for each client.
So in the example above, Q would be evaluated once,
with the results transmitted to N different clients.
* First step towards removing `Table` from query plans
Rework `SourceExpr` to be a logical placeholder for a table,
rather than a table itself.
Make query eval functions take an additional argument, a set of tables.
When evaluating a `SourceExpr` to a table, they will treat the `SourceExpr`
as a reference into the set of sources, and use the referred table.
This commit modifies only the VM crate; modifications to `core` are forthcoming.
It's possible that this commit's scheme for referring to `SourceExpr`s
will need to change,
as currently it forbids duplicate `SourceExpr`s,
which I think might occur during index joins.
* `SourceBuilder` interface for assigning `SourceId`s.
* Integrate new query plan repr into `core`
* Put `DbTable` back in the AST; only `MemTable` is separate
Per review from Joshua, this commit makes `SourceExpr` into an enum
similar to the previous definition, with a `DbTable(DbTable)` variant.
Indirection to a `SourceSet` is imposed only for the `MemTable` variant.
This sould make the PR's overall diff much simpler
(assuming I haven't inadvertently made any changes
in the process of reverting the `DbTable` code paths).
Related to the above, this PR simplifies `SourceSet`.
`SourceSet` now holds a `Vec<Option<MemTable>>`, where previously
it was a transparent newtype around `[Option<Table>]`.
This change eliminates the need for unsafe unsized conversions,
removes `SourceBuilder`,
and causes `SourceSet` to be uniformly consumed by the high-level query eval operators,
where previously `SourceSet`s had to be semi-reusable
because they could contain `DbTable`s.
* Per Mazdak's review, docs!
* Adding an index selector that take in account multi-column indexes (and improve the query! macro)
* move select_best_index to vm/src/expr.rs; get rid of OpCmpIdx
* refactor test best_index
* simplify best_index* tests more
* create_table_multi_index: use ColListBuilder
* move & simplify create_table_multi_index
* simplify assert_index_scan + uses
* remove create_table, twas dead code
* ColumnOpFlat: use SmallVec instead
* simplify ScanIndex
* simplify best_index_range
* Add test for sql + joins + multi-index and fix invalid ambiguos field error
* slightly refactor select_best_index
* remove nonempty dependency
* Add test that actually run the multi-column sql
* Adding benchmark for multi vs many indexes
* simplify create_table_for_test*
* Add comments
* impl new algo for select_best_index + clone less
* improve select_best_index docs
* ScanIndex -> ScanOrIndex
* simplify is_sargable + use smallvec more
* let make_index handle a single ScanOrIndex
* make index stuff more private + remove dead code
* select_best_index: return IndexColumnOp directly; nix ScanOrindex -- this removes an allocation
* do not reconstruct scan argument; avoid heap allocations
* borrow ColList in IndexArgument + avoid temp alloc in is_sargable
* optmize_select: remove Cow from fields_found
* is_sargable: reuse allocation from extract_fields
* rename is_sargable, avoid temp fields_found allocs, simplify optmize_select
* fix subscription benches
* drive-by refactor benches/subscription
* Keep a single benchmark for location
* Squashed commit of the following:
commit e54b09bab2
Author: Mario Montoya <mamcx@elmalabarista.com>
Date: Thu Feb 29 20:19:24 2024 -0500
Correctly show the error for AmbiguousField and simplify the code (#910)
commit 48a205a818
Author: Kim Altintop <kim@eagain.io>
Date: Thu Feb 29 19:32:21 2024 +0100
core: Fix host controller to not replace module if lifecyle hooks failed (#904)
* core: Fix host controller to not replace module if lifecyle hooks failed
Previously, `spawn_module_host` would unconditionally insert the new
module into the controller state, and not remove it if the lifecycle
hooks (`init_database` / `update_database`) returned an error.
This would mean that the module code was replaced with the new one, even
if it should be rejected because the schema was not updated or the init
/ update reducer failed.
Fix this by starting the module, and later "committing" it to the
controller state in two phases.
* Add commentary about database mutations / transactionality
---------
Signed-off-by: Mario Montoya <mamcx@elmalabarista.com>
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
* core: Fix host controller to not replace module if lifecyle hooks failed
Previously, `spawn_module_host` would unconditionally insert the new
module into the controller state, and not remove it if the lifecycle
hooks (`init_database` / `update_database`) returned an error.
This would mean that the module code was replaced with the new one, even
if it should be rejected because the schema was not updated or the init
/ update reducer failed.
Fix this by starting the module, and later "committing" it to the
controller state in two phases.
* Add commentary about database mutations / transactionality
* perf(832): Remove redundant row deduplication in subscriptions
Closes#832.
The database already operates under set semantics,
so unless multiple queries return rows from the same table,
deduplication of the result set is not necessary.
* Rip out all deduplication
---------
Co-authored-by: Noa <coolreader18@gmail.com>
* Remove `row_pk` from client API; hash rows not row_ids on client
This commit removes the `row_pk` field from `message TableRowOperation` in the client API.
This is the beginning of addressing issue #831.
This has several implications for clients:
- Because we no longer have a stable content-addressed ID for each row,
the client is responsible for hashing rows itself.
- This means that generated types must be `Hash + Eq`.
- This means that generated types must use `sats::F32` or `sats::F64`
rather than `f32` or `f64`,
as the former are `Eq + Hash` while the latter are not.
(Aside: it's stupid that Rust floats are not `Eq + Ord + Hash`,
since IEEE-754 defines a total equality and total ordering on floats.)
Exposing `sats::F32` to users is ugly, but I can't think of a better design.
This commit also makes some minor formatting changes to Rust codegen,
since it was emitting code that rustc warned about.
Still outstanding:
- [ ] Remove row_pk from the JSON API.
- [ ] Avoid computing row_pk in the subscription engine.
- [ ] Update other SDKs.
- [ ] C#
- [ ] TypeScript
- [ ] Python
* Combine similar patterns
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
* Remove `row_pk` from JSON messages
* fmt
* SDK: ClientCache keyed on BSATN bytes, not domain types
Per Ingvar's suggestion, this commit makes the client cache
be `HashMap<Vec<u8>, T>`, where the key is a BSATN byte-buffer
containing the serialized representation of the value.
This means that generated structs and enums don't need to be `Hash + Eq`.
The key upside here is that we can revert to using `f32`/`f64`
rather than `sats::F32`/`sats::F64`.
---------
Signed-off-by: Phoebe Goldman <phoebe@goldman-tribe.org>
Co-authored-by: Mazdak Farrokhzad <twingoow@gmail.com>
Closes#890.
This changes the magic constant from 1000 to 3000 rows,
which means if the indexed table in an index join has 3000 rows or less,
we make it the probe table instead.
Zeke Foppa