CQL supports type casting using C-style casts.
For example it's possible to do: `blob_column = (blob)funcReturningInt()`
This functionality is pretty limited, we only allow such casts between types that have a compatible binary representation. Compatible means that the bytes will stay unchanged after the conversion.
This means that it's legal to cast an int to blob (int is just a 4 byte blob), but it's illegal to cast a bigint to int (change 4 bytes -> 8 bytes).
This simplifies things, to cast we can just reinterpret the value as the other type.
Another use of C-style casts are type hints. Sometimes it's impossible to infer the exact type of an expression from the context. In such cases the type can be specified by casting the expression to this type.
For example: `overloadedFunction((int)?)`
Without the cast it would be impossible to guess what should be the bind marker's type. The function is overloaded, so there are many possible argument types. The type hint specifies that the bind marker has type int.
An interesting thing is that such casts don't have to be explicit. CQL allows to put an int value in a place where a blob value is expected and it will be automatically converted without any explicit casting.
---
I started looking at our implementation of casts because of #12900. In there the author expressed the need to specify a type hint for bind marker used to pass the WASM code. It could be either `(text)?` for text WASM, or `(blob)?` for binary WASM. This specific use of type hints wasn't supported because there was no `receiver` and the implementation of `prepare_expression` didn't handle that. Preparing casts without a receiver should be easy to implement - we can infer the type of the expression by looking at the type to which the expression is cast.
But while reading `prepare_expression` for `expr::cast` I noticed that the code there is a bit strange. The implementation prepared the expression to cast using the original `receiver` instead of a receiver with the cast type. This caused some issues because of which casting didn't work as expected.
For example it was possible to do:
```cql
blob_column = (blob)funcReturningInt()
```
But this didn't work at all:
```cql
blob_column = (blob)(int)12323
```
It tried to prepare `untyped_contant(12323)` with a `blob` receiver, which fails.
This makes `expr::cast` useless for casting. Casting when the representation is compatible is already implicit. I couldn't find a single case where adding a cast would change the behavior in any way.
There was some use for it as a type hint to choose a specific overload of a function, but it was worthless for casting.
Cassandra has the same issue, I created a `cql-pytest` test and it showed that we behave in the same way as Cassandra does.
I decided to improve this. By preparing the expression using a receiver with the cast type, `expr::cast` becomes actually useful for casting values. Things like `(blob)(int)12323` now work without any issues.
This diverges from the behavior in Cassandra, but it's an extension, not a breaking incompatibility.
---
This PR improves `prepare_expression` for `expr::cast` in the following ways:
1) Support for more complex casts by preparing the expression using a different receiver. This makes casts like `(blob)(int)123` possible
2) Support preparing `expr::cast` without a receiver. Type inference chooses the cast type as the type of the expression.
3) Add pytest tests for C-style casts
`2)` Is needed for #12900, the other changes is just something I decided to do since I was already working on this piece of code.
Closes#13053
* github.com:scylladb/scylladb:
expr_test: more tests for preparing bind variables with type hints
prepare_expr: implement preparing expr::cast with no receiver
prepare_expr: use :user formatting in cast_prepare_expression
prepare_expr: remove std::get<> in cast_prepare_expression
prepare_expr: improve cast_prepare_expression
prepare_expr: improve readability in cast_prepare_expression
cql-pytest: test expr::cast in test_cast.py
Type inference in cast_prepare_expression was very limited.
Without a receiver it just gave up and said that it can't
infer the type.
It's possible to infer the type - an expression that
casts something to type bigint also has type bigint.
This can be implemented by creating a fake receiver
when the caller didn't specify one.
Type of this fake receiver will be c.type
and c.arg will be prepared using this receiver.
Note that the previous change (changing receiver
to cast_type_receiver in prepare_expression) is required
to keep the behaviour consistent.
Without it we would sometimes prepare c.arg using the
original receiver, and sometimes using a receiver
with type c.type.
Currently it's impossible to test this change
on live code. Every place that uses expr::cast
specifies a receiver.
A unit test is all that can be done at the moment
to ensure correctness.
In the future this functionality will be used in UDFs.
In https://github.com/scylladb/scylladb/pull/12900
it was requested to be able to use a type hint
to specify whether WASM code of the function
will be sent in binary or text form.
The user can convey this by typing
either `(blob)?` or `(text)?`.
In this case there will be no receiver
and type inference would fail.
After this change it will work - it's now possible
to prepare either of those and get an expression
with a known type.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
By default expressions are printed using the {:debug} formatting,
wich is intended for internal use. Error messages should use the
{:user} formatting instead.
cast_prepare_expression uses the default formatting in a few places
that are user facing, so let's change it to use {:user} formatting.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
A few times throughout cast_prepare_expression there's
a line which uses std::get<> to get the raw type of the cast.
`std::get<shared_ptr<cql3_type::raw>>(c.type)`
This is a dangerous thing to do. It might turn out that the variant
holds a different alternative and then it'll start throwing bad_variant_access.
In this case this would happen if someone called cast_prepare_expression
on an expression that is already prepared.
It's possible to modify the code in a way that avoids doing the std::get
altogether.
It makes the code more resilient and gives me a piece of mind.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Preparing expr::cast had some artificial limitations.
Things like this worked:
`blob_col = (blob)funcReturnsInt()`
But this didn't:
`blob_col = (blob)(int)1234`
This is caused by the line:
`prepare_expression(c.arg, db, keyspace, schema_opt, receiver)`
Here the code prepares the expression to be cast using the original
receiver which was passed to cast_prepare_expression.
In the example above this meant that it tried to prepare
untyped_constant(1234) using a receiver with type blob.
This failed because an integer literal is invalid for a blob column.
To me it looks like a mistake. What it should do instead
is prepare the int literal using the type (int) and then
see if int can be cast to blob, by checking if these types
have compatible binary representation.
This can be achieved by using `cast_type_receiver` instead of `receiver`.
Making this small change makes it possible to use the cast
in many situations where it was previously impossible.
The tests have to be updated to reflect the change,
some of them ow deviate from Cassandra, so they have
to be marked scylla_only.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
cast_prepare_expression takes care of preparing expr::cast,
which is responsible for CQL C-style casts.
At the first glance it can be hard to figure out what exactly
does it do, so I added some comments to make things clearer.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
There was a bug in `expr::search_and_replace`.
It doesn't preserve the `order` field of binary_operator.
`order` field is used to mark relations created
using the SCYLLA_CLUSTERING_BOUND.
It is a CQL feature used for internal queries inside Scylla.
It means that we should handle the restriction as a raw
clustering bound, not as an expression in the CQL language.
Losing the SCYLLA_CLUSTERING_BOUND marker could cause issues,
the database could end up selecting the wrong clustering ranges.
Fixes: #13055
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Closes#13056
these warnings are found by Clang-17 after removing
`-Wno-unused-lambda-capture` and '-Wno-unused-variable' from
the list of disabled warnings in `configure.py`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Currently, evaluation of a subscript expression x[y] requires that
x be a column_value, but that's completely artificial. Generalize
it to allow any expression.
This is needed after we transform a LWT IF condition from
"a[x] = y" to "func(a)[x] = y", where func casts a from a
map represention of a list back to a list; but it's also generally
useful.
LWT and some list operations represent lists using a form like
their mutations, so that the mutation list keys can be recovered
and used to update the list. But the evaluation machinery knows
nothing about that, and will return the map-form even though the type
system thinks it is a list.
To handle that, add a utility to rewrite the expression so
that the value is re-serialized into the expected list form. The
rewrite is implemented as a scalar function taking the map form and
returning the list form.
Partial clustering keys can exist in COMPACT STORAGE tables (though they
are exceedingly rare), and when LWT materializes a static row. Harden
extract_column_value() so it is ready for them.
Expression evaluation works with the evaluation_input structure to
compute values. As we move LWT column_condition towards expressions,
we'll start using evaluation_input, so provide this helper to ease
the transition.
Both LWT IF clause and SELECT WHERE clause check that a duration type
isn't used in an ordered comparison, since duration types are unordered
(is 1mo more or less than 30d?). As a first step towards centralizing this
check, move the check from restrictions into prepare. When LWT starts using
prepare, the duplication will be removed.
The error message was changed: the word "slice" is an internal term, and
a comparison does not necessarily have to be in a restriction (which is
also an internal term).
Tests were adjusted.
Compiling a pattern is expensive and so we should try to do it
at prepare time, if the pattern is a constant. Add an optimizer
that looks for such cases and replaces them with a unary function
that embeds the compiled pattern.
This isn't integrated yet with prepare_expr(), since the filtering
code isn't ready for generic expressions. Its first user will be LWT,
which contains the optimization already (filtering had it as well,
but lost it sometime during the expression rewrite).
A unit test is added.
Function call evaluation rejects NULL inputs, unnecssarily. Functions
work well with NULL inputs. Fix by relaxing the check.
This currently has no impact because functions are not evaluated via
expressions, but via selectors.
LWT IF clause interprets equality differently from SQL (and the
rest of CQL): it thinks NULL equals NULL. Currently, it implements
binary operators all by itself so the fact that oper_t::EQ (and
friends) means something else in the rest of the code doesn't
bother it. However, we can't unify the code (in
column_condition.cc) with the rest of expression evaluation if
the meaning changes in different places.
To prepare for this, introduce a null_handling_style field to
binary_operator that defaults to `sql` but can be changed to
`lwt_nulls` to indicate this special semantic.
A few unit tests are added. LWT itself still isn't modified.
Currently, evaluate_binop_sides() returns std::nullopt if either
side is NULL.
Since we wish to to add binary operators that do consider NULL on
each side, make evaluate_binop_sides return the original NULLs
instead (as managed_bytes_opt).
Utimately I think evaluate_binop_sides() should disappear, but before
that we have to improve unset value checking.
We have a cql3::expr::expression::printer wrapper that annotates
an expression with a debug_mode boolean prior to formatting. The
fmt library, however, provides a much simpler alterantive: a custom
format specifier. With this, we can write format("{:user}", expr) for
user-oriented prints, or format("{:debug}", expr) for debug-oriented
prints (if nothing is specified, the default remains debug).
This is done by implementing fmt::formatter::parse() for the
expression type, can using expression::printer internally.
Since sometimes we pass expression element types rather than
the expression variant, we also provide a custom formatter for all
ExpressionElement Types.
Uses for expression::printer are updated to use the nicer syntax. In
one place we eliminate a temporary that is no longer needed since
ExpressionElement:s can be formatted directly.
Closes#12702
`evaluation_inputs` is a struct which contains data needed to evaluate expressions - values of columns, bind variables and other data.
`is_on_of()` is a function used to to evaluate `IN` restrictions. It checks whether the LHS is one of elements on the RHS list.
Generally when evaluating expressions we get the `evaluation_inputs` as an argument and we should pass them along to any functions that evaluate subexpressions.
`is_one_of()` got the inputs as an argument, but didn't pass them along to `equal()`, instead it creates new empty `evaluation_inputs{}` and gives that to `equal()`.
At first [I thought this was a bug](https://github.com/scylladb/scylladb/pull/12356#discussion_r1084300969) - with missing information there could be a crash if `equal()` tried to evaluate an expression with a `bind_variable`.
It turns out that in this particular case `equal()` won't use the `evaluation_inputs` at all. The LHS and RHS passed to it are just constant values, which were already evaluated to serialized bytes before calling `evaluate()`, so there is no bug.
It's still better to pass the inputs argument along if possible. If in the future `equal()` required these inputs for some reason, missing inputs could lead to an unexpected crash.
I couldn't find any tests that would detect this case, so such a bug could stay undetected until an unhappy user finds it because their cluster crashed.
I added some tests to make sure that it's covered from now on.
Closes#12701
* github.com:scylladb/scylladb:
cql-pytest: test filtering using list with bind variable
test/expr_test: test <int_value> IN (123, ?, 456)
cql3: expr: don't pass empty evaluation_inputs in is_one_of
evaluation_inputs is a struct which contains
data needed to evaluate expressions - values
of columns, bind variables and other data.
is_on_of() is a function used to to evaluate
IN restrictions. It checks whether the LHS
is one of elements on the RHS list.
Generally when evaluating expressions we get
the evaluation_inputs{} as an argument and
we should pass them along to any functions
that evaluate subexpressions.
is_one_of() got the inputs as an argument,
but didn't pass them along to equal(),
instead it creates new empty evaluation_inputs{}
and gives that to equal().
At first I thought this was a bug - with missing
information there could be a crash if equal()
tried to evaluate an expression with a bind_variable.
It turns out that in this particular case equal()
won't use the evaluation_inputs{} at all.
The LHS and RHS passed to it are just constant values,
which were already evaluated to serialized bytes
before calling evaluate().
It's still better to pass the inputs argument along
if possible. If in the future equal() required
these inputs for some reason, missing inputs
could lead to an unexpected crash.
I couldn't find any tests that would detect this case,
so such a bug could stay undetected until an unhappy user
finds it because their cluster crashed.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
since format_to() is defined included by both fmt and std namepaces,
without specifying which one to use, we'd fail to build with the
standard library which implements std::format_to(). yes, we are
`using namespace std` somewhere.
this change should address the FTBFS with GCC-13.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
The CQL protocol and specification call for lists with NULLs in
some places. For example, the statement:
```cql
UPDATE tab
SET x = 3
IF y IN (1, 2, NULL)
WHERE pk = 4
```
has a list `(1, 2, NULL)` that contains NULL. Although the syntax is tuple-like, the value is a list;
consider the same statement as a prepared statement:
```cql
UPDATE tab
SET x = :x
IF y IN :y_values
WHERE pk = :pk
```
`:y_values` must have a list type, since the number of elements is unknown.
Currently, this is done with special paths inside LWT that bypass normal
evaluation, but if we want to unify those paths, we must allow NULLs in
lists (except in storage). This series does that.
Closes#12411
* github.com:scylladb/scylladb:
test: materialized view: add test exercising synthetic empty-type columns
cql3: expr: relax evaluate_list() to allow allow NULL elements
types: allow lists with NULL
test: relax NULL check test predicate
cql3, types: validate listlike collections (sets, lists) for storage
types: make empty type deserialize to non-null value
prepare_expression didn't allow to prepare binary_operators.
so it's now implemented.
If prepare_binary_operator is unable to infer
the types it will fail with an exception instead
of returning std::nullopt, but we can live with
that for now.
Preparing binary_operators inside the WHERE
clause is currently more complicated than just
calling prepare_binary_operator. Preparation
of the WHERE clause is done inside statement_restrictions
constructor. It's done by iterating over all binary_operators,
validating them and then preparing. The validation contains
additional checks with custom error messages.
Preparation has to be done after validation,
because otherwise the error messages will change
and some tests will start failing.
Because of that we can't just call prepare_expression
on the WHERE clause yet.
It's still useful to have the ability to prepare
binary_operators using prepare_expression.
In cases where we know that the WHERE clause is valid,
we can just call prepare_expression and be done with it.
Once grammar is fully relaxed the artificial constraints
checked by the validation code will be removed and
it will be possible to prepare the whole WHERE clause
using just prepare_expression.
prepare_expression does a bit more than
prepare_binary_operator. In case where
both sides of the binary_operator are known
it will evaluate the whole binary_operator
to a constant value.
Query analysis code is NOT ready
to encounter constant boolean values inside
the WHERE clause, so for the WHERE we still use
prepare_binary_operator which doesn't
evaluate the binary_operator to a
constant value.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
When preparing a binary operator we first prepare the LHS,
which gives us information about its type and allows
to infer the desired type of RHS.
Then the RHS is prepared with the expectation that it
is compatible with the inferred type.
This is enough for all types of operations apart
from IS NOT NULL.
For IS NOT we should also check that the RHS value
is actually null. It's not enough to check that
RHS is of right type.
Before this change preparing `int_col IS NOT 123`
would end in success, which is wrong.
The missing check doesn't cause any real problems,
it's impossible for the user to produce such input
because the parser will reject it.
Still it's better to have the check because
in the future the grammar might get more relaxed
and the parser could become more generic,
making it possible to write such things.
It would be better to introduce unary_operators,
but that's a bigger change.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
For some reason we passed an empty keyspace name
to prepare_expression when preparing the LHS
of a binary operator.
This doesn't look correct. We have keyspace
name available from the schema_ptr so let's use that.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
prepare_binary_operator takes a schema_ptr,
but it would be useful to take a reference to schema instead.
Every schema_ptr can be easily converted to a reference
so there is no loss of functionality.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Tests are similarly relaxed. A test is added in lwt_test to show
that insertion of a list with NULL is still rejected, though we
allow NULLs in IF conditions.
One test is changed from a list of longs to a list of ints, to
prevent churn in the test helper library.
When we start allowing NULL in lists in some contexts, the exact
location where an error is raised (when it's disallowed) will
change. To prepare for that, relax the exception check to just
ensure the word NULL is there, without caring about the exact
wording.
The CQL binary protocol introduced "unset" values in version 4
of the protocol. Unset values can be bound to variables, which
cause certain CQL fragments to be skipped. For example, the
fragment `SET a = :var` will not change the value of `a` if `:var`
is bound to an unset value.
Unsets, however, are very limited in where they can appear. They
can only appear at the top-level of an expression, and any computation
done with them is invalid. For example, `SET list_column = [3, :var]`
is invalid if `:var` is bound to unset.
This causes the code to be littered with checks for unset, and there
are plenty of tests dedicated to catching unsets. However, a simpler
way is possible - prevent the infiltration of unsets at the point of
entry (when evaluating a bind variable expression), and introduce
guards to check for the few cases where unsets are allowed.
This is what this long patch does. It performs the following:
(general)
1. unset is removed from the possible values of cql3::raw_value and
cql3::raw_value_view.
(external->cql3)
2. query_options is fortified with a vector of booleans,
unset_bind_variable_vector, where each boolean corresponds to a bind
variable index and is true when it is unset.
3. To avoid churn, two compatiblity structs are introduced:
cql3::raw_value{,_view}_vector_with_unset, which can be constructed
from a std::vector<raw_value{,_view/}>, which is what most callers
have. They can also be constructed with explicit unset vectors, for
the few cases they are needed.
(cql3->variables)
4. query_options::get_value_at() now throws if the requested bind variable
is unset. This replaces all the throwing checks in expression evaluation
and statement execution, which are removed.
5. A new query_options::is_unset() is added for the users that can tolerate
unset; though it is not used directly.
6. A new cql3::unset_operation_guard class guards against unsets. It accepts
an expression, and can be queried whether an unset is present. Two
conditions are checked: the expression must be a singleton bind
variable, and at runtime it must be bound to an unset value.
7. The modification_statement operations are split into two, via two
new subclasses of cql3::operation. cql3::operation_no_unset_support
ignores unsets completely. cql3::operation_skip_if_unset checks if
an operand is unset (luckily all operations have at most one operand that
tolerates unset) and applies unset_operation_guard to it.
8. The various sites that accept expressions or operations are modified
to check for should_skip_operation(). This are the loops around
operations in update_statement and delete_statement, and the checks
for unset in attributes (LIMIT and PER PARTITION LIMIT)
(tests)
9. Many unset tests are removed. It's now impossible to enter an
unset value into the expression evaluation machinery (there's
just no unset value), so it's impossible to test for it.
10. Other unset tests now have to be invoked via bind variables,
since there's no way to create an unset cql3::expr::constant.
11. Many tests have their exception message match strings relaxed.
Since unsets are now checked very early, we don't know the context
where they happen. It would be possible to reintroduce it (by adding
a format string parameter to cql3::unset_operation_guard), but it
seems not to be worth the effort. Usage of unsets is rare, and it is
explicit (at least with the Python driver, an unset cannot be
introduced by ommission).
I tried as an alternative to wrap cql3::raw_value{,_view} (that doesn't
recognize unsets) with cql3::maybe_unset_value (that does), but that
caused huge amounts of churn, so I abandoned that in favor of the
current approach.
Closes#12517
The function underlying_type() returns an data_type by value,
but the code assigned it to a reference.
At first I was sure this is an error
(assigning temporary value to a reference), but it turns out
that this is most likely correct due to C++ lifetime
extension rules.
I think it's better to avoid such unituitive tricks.
Assigning to value makes it clearer that the code
is correct and there are no dangling references.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Closes#12485
Now that we don't accept cql protocol version 1 or 2, we can
drop cql_serialization format everywhere, except when in the IDL
(since it's part of the inter-node protocol).
A few functions had duplicate versions, one with and one without
a cql_serialization_format parameter. They are deduplicated.
Care is taken that `partition_slice`, which communicates
the cql_serialization_format across nodes, still presents
a valid cql_serialization_format to other nodes when
transmitting itself and rejects protocol 1 and 2 serialization\
format when receiving. The IDL is unchanged.
One test checking the 16-bit serialization format is removed.
prepare_expression used to throw an error
when encountering a conjunction.
Now it's possible to use prepare_expression
to prepare an expression that contains
conjunctions.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Previously it was impossible to use expr::evaluate()
to get the value of a conjunction of elements
separated by ANDs.
Now it has been implemented.
NULL is treated as an "unkown value" - maybe true maybe false.
`TRUE AND NULL` evaluates to NULL because it might be true but also might be false.
`FALSE AND NULL` evaluates to FALSE because no matter what value NULL acts as, the result will still be FALSE.
Unset and empty values are not allowed.
Usually in CQL the rule is that when NULL occurs in an operation the whole expression
becomes NULL, but here we decided to deviate from this behavior.
Treating NULL as an "unkown value" is the standard SQL way of handing NULLs in conjunctions.
It works this way in MySQL and Postgres so we do it this way as well.
The evaluation short-circuits. Once FALSE is encountered the function returns FALSE
immediately without evaluating any further elements.
It works this way in Postgres as well, for example:
`SELECT true AND NULL AND 1/0 = 0` will throw a division by zero error
but `SELECT false AND 1/0 = 0` will successfully evaluate to FALSE.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Our `null` expression, after the prepare stage, is redundant with a
`constant` expression containing the value NULL.
Remove it. Its role in the unprepared stage is taken over by
untyped_constant, which gains a new type_class enumeration to
represent it.
Some subtleties:
- Usually, handling of null and untyped_constant, or null and constant
was the same, so they are just folded into each other
- LWT "like" operator now has to discriminate between a literal
string and a literal NULL
- prepare and test_assignment were folded into the corresponing
untyped_constant functions. Some care had to be taken to preserve
error messages.
Closes#12118
In ad3d2ee47d, we replaced `bool` as an expression element
(representing a boolean constant) with `constant`. But a comment
and a concept continue to mention it.
Remove the comment and the concept fragment.
Closes#12119
The messages used to contain UNSET_VALUE
in capital letters, but the tests
expect messages with 'unset value'.
Change the message so that it can
match the expected error text in tests.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Originally put braces around the cases because
there were local variables that I didn't want
to be shadowed.
Now there are no variables so the braces
can be removed without any problems.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
When evaluating a binary operation with
operations like EQUAL, LESS_THAN, IN
the logic of the operation is put
in a separate function to keep things clean.
IS_NOT NULL is the only exception,
it has its evaluate implementation
right in the evaluate(binary_operator)
function.
It would be cleaner to have it in
a separate dedicated function,
so it's moved to one.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
There is a more general version of limits()
which takes expressions as both the lhs and rhs
arguments.
There is no need for a specialized overload.
This specialized overload takes a tuple_constructor
as lhs, but we call evaluate() on both sides
of a binary operator before checking equality,
so this won't be useful at all.
Having multiple functions increases the risk
that one of them has a bug, while giving
dubious benfit.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Expressions like:
123 = NULL
NULL = 123
NULL = NULL
NULL != 123
should be tolerated, but evaluate to NULL.
The current code assumes that a binary operator
can only evaluate to a boolean - true or false.
Now a binary operator can also evaluate to NULL.
This should happen in cases when one of the
operator's sides is NULL.
A special class is introduced to represent a value
that can be one of three things: true, false or null.
It's better than using std::optional<bool>,
because optional has implicit conversions to bool
that could cause confusion and bugs.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
There is a more general version of equal()
which takes expressions as both the lhs and rhs
arguments.
There is no need for a specialized overload.
This specialized overload takes a tuple_constructor
as lhs, but we call evaluate() on both sides
of a binary operator before checking equality,
so this won't be useful at all.
Having multiple functions increases the risk
that one of them has a bug, while giving
dubious benfit.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
is_satisfied_by has to check if a binary_operator is satisfied
by some values. It used to be impossible to evaluate
a binary_operator, so is_satisfied had code to check
if its satisfied for a limited number of cases
occuring when filtering queries.
Now evaluate(binary_operator) has been implemented
and is_satisfied_by can use it to check if a binary_operator
evaluates to true.
This is cleaner and reduces code duplication.
Additionally cql tests will test the new evalute() implementation.
There is one special case with token().
When is_satisfied_by sees a restriction on token
it assumes that it's satisfied because it's
sure that these token restrictions were used
to generate partition ranges.
I had to leave this special case in because it's impossible
to evaluate(token). Once this is implemented I will remove
the special case because it's risky and prone to cause
bugs.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
The code to evaluate binary operators
was copied from is_satisfied_by.
is_satisfied_by wasn't able to evaluate
IS NOT NULL restrictions, so when such restriction
is encountered it throws an exception.
Implement proper handling for IS NOT NULL binary operators.
The switch ensures that all variants of oper_t are handled,
otherwise there would be a compilation error.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
