GitHub – scylladb/scylladb: NoSQL data store using the seastar framework, compatible with Apache Cassandra

Let's remove `expr::token` and replace all of its functionality with `expr::function_call`.

`expr::token` is a struct whose job is to represent a partition key token.
The idea is that when the user types in `token(p1, p2) < 1234`, this will be internally represented as an expression which uses `expr::token` to represent the `token(p1, p2)` part.

The situation with `expr::token` is a bit complicated.
On one hand side it's supposed to represent the partition token, but sometimes it's also assumed that it can represent a generic call to the `token()` function, for example `token(1, 2, 3)` could be a `function_call`, but it could also be `expr::token`.

The query planning code assumes that each occurence of expr::token
represents the partition token without checking the arguments.
Because of this allowing `token(1, 2, 3)` to be represented as `expr::token` is dangerous - the query planning might think that it is `token(p1, p2, p3)` and plan the query based on this, which would be wrong.

Currently `expr::token` is created only in one specific case.
When the parser detects that the user typed in a restriction which has a call to `token` on the LHS it generates `expr::token`.
In all other cases it generates an `expr::function_call`.
Even when the `function_call` represents a valid partition token, it stays a `function_call`. During preparation there is no check to see if a `function_call` to `token` could be turned into `expr::token`. This is a bit inconsistent - sometimes `token(p1, p2, p3)` is represented as `expr::token` and the query planner handles that, but sometimes it might be represented as `function_call`, which the query planner doesn't handle.

There is also a problem because there's a lot of code duplication between a `function_call` and `expr::token`.
All of the evaluation and preparation is the same for `expr::token` as it's for a `function_call` to the token function.
Currently it's impossible to evaluate `expr::token` and preparation has some flaws, but implementing it would basically consist of copy-pasting the corresponding code from token `function_call`.

One more aspect is multi-table queries.
With `expr::token` we turn a call to the `token()` function into a struct that is schema-specific.
What happens when a single expression is used to make queries to multiple tables? The schema is different, so something that is represented as `expr::token` for one schema would be represented as `function_call` in the context of a different schema.
Translating expressions to different tables would require careful manipulation to convert `expr::token` to `function_call` and vice versa. This could cause trouble for index queries.

Overall I think it would be best to remove `expr::token`.

Although having a clear marker for the partition token is sometimes nice for query planning, in my opinion the pros are outweighted by the cons.
I'm a big fan of having a single way to represent things, having two separate representations of the same thing without clear boundaries between them causes trouble.

Instead of having both `expr::token` and `function_call` we can just have the `function_call` and check if it represents a partition token when needed.

Refs: #12906
Refs: #12677
Closes
: #12905

Closes
 #13480

* github.com:scylladb/scylladb:
  cql3: remove expr::token
  cql3: keep a schema in visitor for extract_clustering_prefix_restrictions
  cql3: keep a schema inside the visitor for extract_partition_range
  cql3/prepare_expr: make get_lhs_receiver handle any function_call
  cql3/expr: properly print token function_call
  expr_test: use unresolved_identifier when creating token
  cql3/expr: split possible_lhs_values into column and token variants
  cql3/expr: fix error message in possible_lhs_values
  cql3: expr: reimplement is_satisfied_by() in terms of evaluate()
  cql3/expr: add a schema argument to expr::replace_token
  cql3/expr: add a comment for expr::has_partition_token
  cql3/expr: add a schema argument to expr::has_token
  cql3: use statement_restrictions::has_token_restrictions() wherever possible
  cql3/expr: add expr::is_partition_token_for_schema
  cql3/expr: add expr::is_token_function
  cql3/expr: implement preparing function_call without a receiver
  cql3/functions: make column family argument optional in functions::get
  cql3/expr: make it possible to prepare expr::constant
  cql3/expr: implement test_assignment for column_value
  cql3/expr: implement test_assignment for expr::constant

1cefb66