Fix few logical codegen issues and add fuzz tests for logical
expressions
- Right now Limbo fails to recognize `false` constant in case when any
unary operator is used on the AST path. This PR add unary operator
option in the rewrite code and handle such cases properly.
```sql
limbo> SELECT NOT FALSE;
× Parse error: no such column: FALSE - should this be a string literal in single-quotes?
```
- `ifnull` implementation produced incorrect codegen due to "careless"
management of registers
```
limbo> SELECT ifnull(0, NOT 0)
[NULL here]
```
- `like` implementation produced incorrect codegen due to "careless"
management of registers
```
limbo> SELECT like('a%', 'a') = 1;
thread 'main' panicked at core/vdbe/mod.rs:1902:41:
internal error: entered unreachable code: Like on non-text registers
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
Depends on https://github.com/tursodatabase/limbo/pull/867 (need
`GrammarGenerator` from this branch)
Closes#869
This PR fixes a bug when index search used incorrect operator if index
column were the "rhs" in the expression (not "lhs" as usual, e.g.
`SELECT * FROM t WHERE 1 < rowid_alias`)
Reviewed-by: Jussi Saurio (@jussisaurio)
Closes#870
This PR fixes implementation of binary shift right/left instructions.
Before there were a minor incompatibility between limbo and sqlite
implementation in case when right shift second argument were more than
64 and first argument were negative. As sqlite implementation of right
binary shift is sign-extended - so `-1` should be returned in such case
when limbo returned zero.
This PR fixes this bug and also introduce a fuzz tests for arithemtic
expressions. This fuzz test were written with a help of
`GrammarGenerator` which allows to easily define probabilistic context-
free grammar and then later sample random strings from it.
Closes#867
# The purpose of this PR
- Current implementation forces users to use logback as their logging
framework
# Changes
- Only add abstraction layer for loggin(which is slf4j in this case)
- In tests, use logback to log out messages(this doesn't affect the
users)
# References
https://github.com/tursodatabase/limbo/issues/615Closes#863
# Purpose of this PR
- When loading native libraries, search jar path as well
- When packaging into a jar file, add native libraries under `/lib`
## How sqlite-jdbc packages their jar file
> Our SQLiteJDBC library requires no configuration since native
libraries for major OSs, including Windows, macOS, Linux etc., are
assembled into a single JAR (Java Archive) file.
=> sqlite-jdbc also packages all major OSs native libraries into a
single JAR
# Changes
- Add build commands in Makefile to build and publish locally
- Load native libraries from `/lib` under jar path
- Add java example under `/example`
# TODO
- Publish to maven central. I might need some help from maintainers.
- We can do better by not adding all the native libraries for every OSs
into the jar(as things can get big, though in compared to JVM, it's
relatively small). We can build for independent OSs and upload the
native libraries somewhere and let users download it and place the
native libraries under system path(which their java apps can discover).
- Or maybe introduce AOT(not sure how to)
# Reference
- [Issue](https://github.com/tursodatabase/limbo/issues/615)
- Now we can do something like this
Closes#862
Fix panic in case when table has row with rowid equals to `-1`
(`=u64::max`)
```sql
limbo> CREATE TABLE t(x INTEGER PRIMARY KEY)
limbo> INSERT INTO t VALUES (-1)
limbo> INSERT INTO t VALUES (NULL);
thread 'main' panicked at core/vdbe/mod.rs:2499:21:
attempt to add with overflow
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
Reviewed-by: Pere Diaz Bou <pere-altea@homail.com>
Closes#868
We transform all JOIN conditions into WHERE clause terms in the query
planner. The JoinAwareConditionExpr name tries to make that point, but I
think it makes things more confusing. Let's call it WhereTerm (suggested
by Jussi).
## Simplify bookkeeping of referenced tables in the query planner
This PR refactors the way we track referenced tables and associated
planner operations related to them (scans, index searches, etc).
## The problem with what we currently have:
- We have a tree data structure called `SourceOperator` which is either
a `Join`, `Scan`, `Search`, `Subquery` or `Nothing`.
```rust
/**
A SourceOperator is a Node in the query plan that reads data from a table.
*/
#[derive(Clone, Debug)]
pub enum SourceOperator {
// Join operator
// This operator is used to join two source operators.
// It takes a left and right source operator, a list of predicates to evaluate,
// and a boolean indicating whether it is an outer join.
Join {
id: usize,
left: Box<SourceOperator>,
right: Box<SourceOperator>,
predicates: Option<Vec<ast::Expr>>,
outer: bool,
using: Option<ast::DistinctNames>,
},
// Scan operator
// This operator is used to scan a table.
// It takes a table to scan and an optional list of predicates to evaluate.
// The predicates are used to filter rows from the table.
// e.g. SELECT * FROM t1 WHERE t1.foo = 5
// The iter_dir are uset to indicate the direction of the iterator.
// The use of Option for iter_dir is aimed at implementing a conservative optimization strategy: it only pushes
// iter_dir down to Scan when iter_dir is None, to prevent potential result set errors caused by multiple
// assignments. for more detailed discussions, please refer to https://github.com/penberg/limbo/pull/376
Scan {
id: usize,
table_reference: TableReference,
predicates: Option<Vec<ast::Expr>>,
iter_dir: Option<IterationDirection>,
},
// Search operator
// This operator is used to search for a row in a table using an index
// (i.e. a primary key or a secondary index)
Search {
id: usize,
table_reference: TableReference,
search: Search,
predicates: Option<Vec<ast::Expr>>,
},
Subquery {
id: usize,
table_reference: TableReference,
plan: Box<SelectPlan>,
predicates: Option<Vec<ast::Expr>>,
},
// Nothing operator
// This operator is used to represent an empty query.
// e.g. SELECT * from foo WHERE 0 will eventually be optimized to Nothing.
Nothing {
id: usize,
},
}
```
- Logically joins are a tree, but this is at least marginally bad for
performance because each `Join` has two boxed child operators, and so
e.g. for a 3-table query you have, for example, 3 `Scan` nodes and then
2 `Join` nodes.
- There are other bigger problems too, though, related to code
structure. We have been carrying around a separate vector of
`referenced_tables` that columns can refer to by index:
```rust
/// A query plan has a list of TableReference objects, each of which represents a table or subquery.
#[derive(Clone, Debug)]
pub struct TableReference {
/// Table object, which contains metadata about the table, e.g. columns.
pub table: Table,
/// The name of the table as referred to in the query, either the literal name or an alias e.g. "users" or "u"
pub table_identifier: String,
/// The index of this reference in the list of TableReference objects in the query plan
/// The reference at index 0 is the first table in the FROM clause, the reference at index 1 is the second table in the FROM clause, etc.
/// So, the index is relevant for determining when predicates (WHERE, ON filters etc.) should be evaluated.
pub table_index: usize,
/// The type of the table reference, either BTreeTable or Subquery
pub reference_type: TableReferenceType,
}
```
- `referenced_tables` is used because SQLite joins are an `n^tables_len`
nested loop, and we need to figure out during which loop to evaluate a
condition expression. A lot of plumbing in the current code exists for
this, e.g. "pushing predicates" in `optimizer` even though "predicate
pushdown" as a query planner concept is an _optimization_, but in our
current system the "pushdown" is really a _necessity_ to move the
condition expressions to the correct `SourceOperator::predicates` vector
so that they are evaluated at the right point.
- `referenced_tables` is also used to map identifiers in the query to
the correct table, e.g. 'foo' `SELECT foo FROM users` becomes an
`ast::Expr::Column { table: 0, .. }` if `users` is the first table in
`referenced_tables`.
In addition to this, we ALSO had a `TableReferenceType` separately for
checking whether the upper-level query is reading from a BTree table or
a Subquery.
```rust
/// The type of the table reference, either BTreeTable or Subquery
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum TableReferenceType {
/// A BTreeTable is a table that is stored on disk in a B-tree index.
BTreeTable,
/// A subquery.
Subquery {
/// The index of the first register in the query plan that contains the result columns of the subquery.
result_columns_start_reg: usize,
},
}
```
...even though we already have an `Operator::Subquery` that should be
able to encode this information, but doesn't, because it's a tree and we
refer to things by index in `referenced_tables`.
### Why this is especially stupid
`SourceOperator` and `TableReference` are basically just two
representations of the same thing, one in tree format and another in
vector format. `SourceOperator` even carries around its own copy of
`TableReference`, even though we ALSO have `referenced_tables:
Vec<TableReference>` 🤡
Note that I'm allowed to call the existing code stupid because I wrote
it.
## What we can do instead
Basically, we can just fold the concerns from `SourceOperator` into
`TableReference` and have a list of those in the query plan, one per
table, in loop order (outermost loop is 0, and so on).
Funnily enough, when Limbo had very very few features we used to have a
Vec of LoopInfos similarly, obviously with a lot less information than
now, but for SQLite it's probably the right abstraction. :)
```rust
pub struct SelectPlan {
/// List of table references in loop order, outermost first.
pub table_references: Vec<TableReference>,
...etc...
}
/// A table reference in the query plan.
/// For example, SELECT * FROM users u JOIN products p JOIN (SELECT * FROM users) sub
/// has three table references:
/// 1. operation=Scan, table=users, table_identifier=u, reference_type=BTreeTable, join_info=None
/// 2. operation=Scan, table=products, table_identifier=p, reference_type=BTreeTable, join_info=Some(JoinInfo { outer: false, using: None }),
/// 3. operation=Subquery, table=users, table_identifier=sub, reference_type=Subquery, join_info=None
#[derive(Debug, Clone)]
pub struct TableReference {
/// The operation that this table reference performs.
pub op: Operation,
/// Table object, which contains metadata about the table, e.g. columns.
pub table: Table,
/// The name of the table as referred to in the query, either the literal name or an alias e.g. "users" or "u"
pub identifier: String,
/// The join info for this table reference if it is the right side of a join (which all except the first table reference have)
pub join_info: Option<JoinInfo>,
}
```
And we keep the "operation" part from the "operator", but in a simple
form:
```rust
pub enum Operation {
Scan {
iter_dir: Option<IterationDirection>,
},
Search(Search),
Subquery {
plan: Box<SelectPlan>,
result_columns_start_reg: usize
},
}
```
Now we don't need to carry around both the operator tree and
`Vec<TableReference>`, because they are the same thing. If something
refers to the `n'th table`, it is just `plan.table_references[n]`.
We also don't need to recurse through the operator tree and usually we
can just loop from outermost table to innermost table.
---
### Handling the "where to evaluate a condition expression" problem
You can see I've also removed the `predicates` vector from `Scan` and
friends. Previously each `SourceOperator` had a vector of `predicates`
so that we knew at which loop depth to evaluate a condition. Now we
align more with what SQLite does -- it puts all the conditions, even the
join conditions, in the `WHERE` clause and adds extra metadata to them:
```rust
/// In a query plan, WHERE clause conditions and JOIN conditions are all folded into a vector of JoinAwareConditionExpr.
/// This is done so that we can evaluate the conditions at the correct loop depth.
/// We also need to keep track of whether the condition came from an OUTER JOIN. Take this example:
/// SELECT * FROM users u LEFT JOIN products p ON u.id = 5.
/// Even though the condition only refers to 'u', we CANNOT evaluate it at the users loop, because we need to emit NULL
/// values for the columns of 'p', for EVERY row in 'u', instead of completely skipping any rows in 'u' where the condition is false.
#[derive(Debug, Clone)]
pub struct JoinAwareConditionExpr {
/// The original condition expression.
pub expr: ast::Expr,
/// Is this condition originally from an OUTER JOIN?
/// If so, we need to evaluate it at the loop of the right table in that JOIN,
/// regardless of which tables it references.
/// We also cannot e.g. short circuit the entire query in the optimizer if the condition is statically false.
pub from_outer_join: bool,
/// The loop index where to evaluate the condition.
/// For example, in `SELECT * FROM u JOIN p WHERE u.id = 5`, the condition can already be evaluated at the first loop (idx 0),
/// because that is the rightmost table that it references.
pub eval_at_loop: usize,
}
```
### Final notes
I've been wanting to make this refactor for a long time now, but the
last straw was when I was making a PR trying to reduce some of the
massive amount of allocations happening in the read path currently, and
I got stuck because of this Operator + referenced_tables shit getting in
the way constantly. So I decided I wanted to get it over with now.
The PR is again very big, but I've artificially split it up into commits
that don't individually compile but at least separate the changes a bit
for you, dear reader.
Closes#853
We don't need `push_predicates()` because that never REALLY was a predicate
pushdown optimization -- it just pushed WHERE clause condition expressions
into the correct SourceOperator nodes in the tree.
Now that we don't have a SourceOperator tree anymore and we keep the conditions
in the WHERE clause instead, we don't need to "push" anything anymore. Leaves
room for ACTUAL predicate pushdown optimizations later :)
We also don't need any weird bitmask stuff anymore, and perhaps we never did,
to determine where conditions should be evaluated.
Now that we do not have a tree of SourceOperators but rather
a Vec of TableReferences, we can just use loops instead of
recursion for handling the main query loop.
- use new TableReference and JoinAwareConditionExpr
- add utilities for determining at which loop depth a
WHERE condition should be evaluated, now that "operators"
do not carry condition expressions inside them anymore.
- Get rid of SourceOperator tree
- Make plan have a Vec of TableReference, and TableReference now
contains the information from the old SourceOperator.
- Remove `predicates` (conditions) from Table References -- put
everything in the WHERE clause like SQLite, and attach metadata
to the where clause expressions with JoinAwareConditionExpr struct.
- Refactor select_star() to be simpler now that we use a vec, not a tree
This PR adjust emitted instructions for expressions which include `IS` /
`IS NOT` operators (support for them in the conditions were added in the
#847)
Reviewed-by: Glauber Costa (@glommer)
Closes#857
This PR remove manual null-comparison optimization (introduced in #847)
which replace `Eq`/`Ne` instructions with explicit `IsNull`/`NotNull`.
There are few factors for this change:
1. Before, manual optimization were incorrect because it ignored
`jump_if_condition_is_true` flag which is important to properly build
logical condition evaluation
2. Manual optimization covered all scenarios in test cases and scenarios
when both sides are non trivial expressions were not covered by tests
3. Limbo already mark literals in the initial emitted bytecode as
constants and will evaluate and store them only once - so performance
difference from manual optimization seems very minor to me (but I am
wrong with high probability)
4. I think (but again, I am wrong with high probability) that such
replacement can be done in the generic optimizator layer instead of
manually encode them in the first emit phase
Fixes#850
Reviewed-by: Glauber Costa (@glommer)
Closes#856
