Merge 'cql3: expr: unify left-hand-side and right-hand-side of binary_operator prepares' from Avi Kivity

Currently, preparing the left-hand-side of a binary operator and the
right-hand-side use different code paths. The left-hand-side derives
the type of the expression from the expression itself, while the
right-hand-side imposes the type on the expression (allowing the types
of bind variables to be inferred).

This series unifies the two, by making the imposed type (the "receiver")
optional, and by allowing prepare to fail gracefully if we were not able
to infer the type. The old prepare_binop_lhs() is removed and replaced
with prepare_expression, already used for the right hand side.

There is one step remaining, and that is to replace prepare_binary_operator
with prepare_expression, but that is more involved and is left for a follow-up.

Closes #10709

* github.com:scylladb/scylla:
  cql3: expr: drop prepare_binop_lhs()
  cql3: expr: move implementation of prepare_binop_lhs() to try_prepare_expression()
  cql3: expr: use recursive descent when preparing subscripts
  cql3: expr: allow prepare of tuple_constructor with no receiver
  cql3: expr: drop no longer used printable_relation parameter from prepare_binop_lhs()
  cql3: expr: print only column name when failing to resolve column
  cql3: expr: pass schema to prepare_expression
  cql3: expr: prepare_binary_operator: drop unused argument ctx
  cql3: expr: stub type inference for prepare_expression
  cql3: expr: introduce type_of() to fetch the type of an expression
  cql3: expr: keep type information in casts
  cql3: expr: add type field to subscript, field_selection, and null expressions
  cql3: expr: cast: use data_type instead of cql3_type for the prepared form

cql3/attributes.cc

@@ -123,15 +123,15 @@ std::unique_ptr<attributes> attributes::raw::prepare(data_dictionary::database d
     std::optional<expr::expression> ts, ttl, to;
 
     if (timestamp.has_value()) {
-        ts = prepare_expression(*timestamp, db, ks_name, timestamp_receiver(ks_name, cf_name));
+        ts = prepare_expression(*timestamp, db, ks_name, nullptr, timestamp_receiver(ks_name, cf_name));
     }
 
     if (time_to_live.has_value()) {
-        ttl = prepare_expression(*time_to_live, db, ks_name, time_to_live_receiver(ks_name, cf_name));
+        ttl = prepare_expression(*time_to_live, db, ks_name, nullptr, time_to_live_receiver(ks_name, cf_name));
     }
 
     if (timeout.has_value()) {
-        to = prepare_expression(*timeout, db, ks_name, timeout_receiver(ks_name, cf_name));
+        to = prepare_expression(*timeout, db, ks_name, nullptr, timeout_receiver(ks_name, cf_name));
     }
 
     return std::unique_ptr<attributes>{new attributes{std::move(ts), std::move(ttl), std::move(to)}};

cql3/column_condition.cc

@@ -298,13 +298,13 @@ column_condition::raw::prepare(data_dictionary::database db, const sstring& keys
             throw exceptions::invalid_request_exception(
                     format("Unsupported collection type {} in a condition with element access", ctype->cql3_type_name()));
         }
-        collection_element_expression = prepare_expression(*_collection_element, db, keyspace, element_spec);
+        collection_element_expression = prepare_expression(*_collection_element, db, keyspace, nullptr, element_spec);
     }
 
     if (is_compare(_op)) {
         validate_operation_on_durations(*receiver.type, _op);
         return column_condition::condition(receiver, std::move(collection_element_expression),
-                prepare_expression(*_value, db, keyspace, value_spec), nullptr, _op);
+                prepare_expression(*_value, db, keyspace, nullptr, value_spec), nullptr, _op);
     }
 
     if (_op == expr::oper_t::LIKE) {
@@ -313,14 +313,14 @@ column_condition::raw::prepare(data_dictionary::database db, const sstring& keys
             // Pass matcher object
             const sstring& pattern = literal_term->raw_text;
             return column_condition::condition(receiver, std::move(collection_element_expression),
-                    prepare_expression(*_value, db, keyspace, value_spec),
+                    prepare_expression(*_value, db, keyspace, nullptr, value_spec),
                     std::make_unique<like_matcher>(bytes_view(reinterpret_cast<const int8_t*>(pattern.data()), pattern.size())),
                     _op);
         } else {
             // Pass through rhs value, matcher object built on execution
             // TODO: caller should validate parametrized LIKE pattern
             return column_condition::condition(receiver, std::move(collection_element_expression),
-                    prepare_expression(*_value, db, keyspace, value_spec), nullptr, _op);
+                    prepare_expression(*_value, db, keyspace, nullptr, value_spec), nullptr, _op);
         }
     }
 
@@ -332,14 +332,14 @@ column_condition::raw::prepare(data_dictionary::database db, const sstring& keys
         assert(_in_values.empty());
         auto in_name = ::make_shared<column_identifier>(format("in({})", value_spec->name->text()), true);
         lw_shared_ptr<column_specification> in_list_receiver = make_lw_shared<column_specification>(value_spec->ks_name, value_spec->cf_name, in_name, list_type_impl::get_instance(receiver.type, false));
-        expr::expression multi_item_term = prepare_expression(*_in_marker, db, keyspace, in_list_receiver);
+        expr::expression multi_item_term = prepare_expression(*_in_marker, db, keyspace, nullptr, in_list_receiver);
         return column_condition::in_condition(receiver, collection_element_expression, std::move(multi_item_term), {});
     }
     // Both _in_values and in _in_marker can be missing in case of empty IN list: "a IN ()"
     std::vector<expr::expression> terms;
     terms.reserve(_in_values.size());
     for (auto&& value : _in_values) {
-        terms.push_back(prepare_expression(value, db, keyspace, value_spec));
+        terms.push_back(prepare_expression(value, db, keyspace, nullptr, value_spec));
     }
     return column_condition::in_condition(receiver, std::move(collection_element_expression),
                                           std::nullopt, std::move(terms));

cql3/expr/expression.cc

@@ -1717,8 +1717,14 @@ constant evaluate(const expression& e, const query_options& options) {
         [](const column_mutation_attribute&) -> constant {
             on_internal_error(expr_logger, "Can't evaluate a column_mutation_attribute");
         },
-        [](const cast&) -> constant {
-            on_internal_error(expr_logger, "Can't evaluate a cast");
+        [&](const cast& c) -> constant {
+            auto ret = evaluate(c.arg, options);
+            auto type = std::get_if<data_type>(&c.type);
+            if (!type) {
+                on_internal_error(expr_logger, "attempting to evaluate an unprepared cast");
+            }
+            ret.type = *type;
+            return ret;
         },
         [](const field_selection&) -> constant {
             on_internal_error(expr_logger, "Can't evaluate a field_selection");
@@ -2317,5 +2323,65 @@ size_t count_if(const expression& e, const noncopyable_function<bool (const bina
     return ret;
 }
 
+data_type
+type_of(const expression& e) {
+    return visit(overloaded_functor{
+        [] (const conjunction& e) {
+            return boolean_type;
+        },
+        [] (const binary_operator& e) {
+            // All our binary operators are relations
+            return boolean_type;
+        },
+        [] (const column_value& e) {
+            return e.col->type;
+        },
+        [] (const token& e) {
+            return long_type;
+        },
+        [] (const unresolved_identifier& e) -> data_type {
+            on_internal_error(expr_logger, "evaluating type of unresolved_identifier");
+        },
+        [] (const column_mutation_attribute& e) {
+            switch (e.kind) {
+            case column_mutation_attribute::attribute_kind::writetime:
+                return long_type;
+            case column_mutation_attribute::attribute_kind::ttl:
+                return int32_type;
+            }
+            on_internal_error(expr_logger, "evaluating type of illegal column mutation attribute kind");
+        },
+        [] (const function_call& e) {
+            return std::visit(overloaded_functor{
+                [] (const functions::function_name& unprepared) -> data_type {
+                    on_internal_error(expr_logger, "evaluating type of unprepared function call");
+                },
+                [] (const shared_ptr<functions::function>& f) {
+                    return f->return_type();
+                },
+            }, e.func);
+        },
+        [] (const bind_variable& e) {
+            return e.receiver->type;
+        },
+        [] (const untyped_constant& e) -> data_type {
+            on_internal_error(expr_logger, "evaluating type of untyped_constant call");
+        },
+        [] (const cast& e) {
+            return std::visit(overloaded_functor{
+                [] (const shared_ptr<cql3_type::raw>& unprepared) -> data_type {
+                    on_internal_error(expr_logger, "evaluating type of unprepared cast");
+                },
+                [] (const data_type& t) {
+                    return t;
+                },
+            }, e.type);
+        },
+        [] (const ExpressionElement auto& e) -> data_type {
+            return e.type;
+        }
+    }, e);
+}
+
 } // namespace expr
 } // namespace cql3

cql3/expr/expression.hh

@@ -213,6 +213,7 @@ struct column_value {
 struct subscript {
     expression val;
     expression sub;
+    data_type type; // may be null before prepare
 };
 
 /// Gets the subscripted column_value out of the subscript.
@@ -309,15 +310,17 @@ struct function_call {
 
 struct cast {
     expression arg;
-    std::variant<cql3_type, shared_ptr<cql3_type::raw>> type;
+    std::variant<data_type, shared_ptr<cql3_type::raw>> type;
 };
 
 struct field_selection {
     expression structure;
     shared_ptr<column_identifier_raw> field;
+    data_type type; // may be null before prepare
 };
 
 struct null {
+    data_type type; // may be null before prepare
 };
 
 struct bind_variable {
@@ -613,11 +616,12 @@ extern expression replace_token(const expression&, const column_definition*);
 extern expression search_and_replace(const expression& e,
         const noncopyable_function<std::optional<expression> (const expression& candidate)>& replace_candidate);
 
-extern expression prepare_expression(const expression& expr, data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver);
+extern expression prepare_expression(const expression& expr, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver);
+std::optional<expression> try_prepare_expression(const expression& expr, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver);
 
 // Prepares a binary operator received from the parser.
 // Does some basic type checks but no advanced validation.
-extern binary_operator prepare_binary_operator(const binary_operator& binop, data_dictionary::database db, schema_ptr schema, prepare_context& ctx);
+extern binary_operator prepare_binary_operator(const binary_operator& binop, data_dictionary::database db, schema_ptr schema);
 
 
 /**
@@ -653,6 +657,8 @@ std::vector<expression> extract_single_column_restrictions_for_column(const expr
 
 std::optional<bool> get_bool_value(const constant&);
 
+data_type type_of(const expression& e);
+
 // Takes a prepared expression and calculates its value.
 // Evaluates bound values, calls functions and returns just the bytes and type.
 constant evaluate(const expression& e, const query_options&);

cql3/expr/prepare_expr.cc

@@ -24,6 +24,43 @@
 
 namespace cql3::expr {
 
+static const column_value resolve_column(const unresolved_identifier& col_ident, const schema& schema);
+
+static
+lw_shared_ptr<column_specification>
+column_specification_of(const expression& e) {
+    return visit(overloaded_functor{
+        [] (const column_value& cv) {
+            return cv.col->column_specification;
+        },
+        [&] (const ExpressionElement auto& other) {
+            auto type = type_of(e);
+            if (!type) {
+                throw exceptions::invalid_request_exception(fmt::format("cannot infer type of {}", e));
+            }
+            // Fake out a column_identifier
+            //
+            // FIXME: come up with something better
+            // This works for now because the we only call this when preparing
+            // a subscript, and the grammar only allows column_values to be subscripted.
+            // So we never end up in this branch. In case we do, we'll see the internal
+            // representation of the expression, rather than what the user typed in.
+            //
+            // The correct fix is to augment expressions with a source_location member so
+            // we can just point at the line and column (and quote the text) of the expression
+            // we're naming. As an example, if we allow
+            //
+            //    WHERE {'a': 3, 'b': 5}[19.5] = 3
+            //
+            // then the column_identifier should be "key type of {'a': 3, 'b': 5}" - it
+            // doesn't identify a column but some subexpression that we're using to infer the
+            // type of the "19.5" (and failing).
+            auto col_id = ::make_shared<column_identifier>(fmt::format("{}", e), true);
+            return make_lw_shared<column_specification>("", "", std::move(col_id), std::move(type));
+        }
+    }, e);
+}
+
 static
 lw_shared_ptr<column_specification>
 usertype_field_spec_of(const column_specification& column, size_t field) {
@@ -70,8 +107,11 @@ usertype_constructor_test_assignment(const usertype_constructor& u, data_diction
 }
 
 static
-expression
-usertype_constructor_prepare_expression(const usertype_constructor& u, data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver) {
+std::optional<expression>
+usertype_constructor_prepare_expression(const usertype_constructor& u, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver) {
+    if (!receiver) {
+        return std::nullopt; // cannot infer type from {field: value}
+    }
     usertype_constructor_validate_assignable_to(u, db, keyspace, *receiver);
     auto&& ut = static_pointer_cast<const user_type_impl>(receiver->type);
     bool all_terminal = true;
@@ -88,7 +128,7 @@ usertype_constructor_prepare_expression(const usertype_constructor& u, data_dict
             raw = iraw->second;
             ++found_values;
         }
-        expression value = prepare_expression(raw, db, keyspace, usertype_field_spec_of(*receiver, i));
+        expression value = prepare_expression(raw, db, keyspace, schema_opt, usertype_field_spec_of(*receiver, i));
 
         if (!is<constant>(value)) {
             all_terminal = false;
@@ -196,8 +236,12 @@ map_test_assignment(const collection_constructor& c, data_dictionary::database d
 }
 
 static
-expression
-map_prepare_expression(const collection_constructor& c, data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver) {
+std::optional<expression>
+map_prepare_expression(const collection_constructor& c, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver) {
+    if (!receiver) {
+        // TODO: It is possible to infer the type of a map from the types of the key/value pairs
+        return std::nullopt;
+    }
     map_validate_assignable_to(c, db, keyspace, *receiver);
 
     auto key_spec = maps::key_spec_of(*receiver);
@@ -210,8 +254,8 @@ map_prepare_expression(const collection_constructor& c, data_dictionary::databas
         if (entry_tuple.elements.size() != 2) {
             on_internal_error(expr_logger, "map element is not a tuple of arity 2");
         }
-        expression k = prepare_expression(entry_tuple.elements[0], db, keyspace, key_spec);
-        expression v = prepare_expression(entry_tuple.elements[1], db, keyspace, value_spec);
+        expression k = prepare_expression(entry_tuple.elements[0], db, keyspace, schema_opt, key_spec);
+        expression v = prepare_expression(entry_tuple.elements[1], db, keyspace, schema_opt, value_spec);
 
         // Check if one of values contains a nonpure function
         if (!is<constant>(k) || !is<constant>(v)) {
@@ -287,8 +331,12 @@ set_test_assignment(const collection_constructor& c, data_dictionary::database d
 }
 
 static
-expression
-set_prepare_expression(const collection_constructor& c, data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver) {
+std::optional<expression>
+set_prepare_expression(const collection_constructor& c, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver) {
+    if (!receiver) {
+        // TODO: It is possible to infer the type of a set from the types of the values
+        return std::nullopt;
+    }
     set_validate_assignable_to(c, db, keyspace, *receiver);
 
     if (c.elements.empty()) {
@@ -319,7 +367,7 @@ set_prepare_expression(const collection_constructor& c, data_dictionary::databas
     bool all_terminal = true;
     for (auto& e : c.elements)
     {
-        expression elem = prepare_expression(e, db, keyspace, value_spec);
+        expression elem = prepare_expression(e, db, keyspace, schema_opt, value_spec);
 
         if (!is<constant>(elem)) {
             all_terminal = false;
@@ -383,8 +431,12 @@ list_test_assignment(const collection_constructor& c, data_dictionary::database
 
 
 static
-expression
-list_prepare_expression(const collection_constructor& c, data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver) {
+std::optional<expression>
+list_prepare_expression(const collection_constructor& c, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver) {
+    if (!receiver) {
+        // TODO: It is possible to infer the type of a list from the types of the key/value pairs
+        return std::nullopt;
+    }
     list_validate_assignable_to(c, db, keyspace, *receiver);
 
     // In Cassandra, an empty (unfrozen) map/set/list is equivalent to the column being null. In
@@ -400,7 +452,7 @@ list_prepare_expression(const collection_constructor& c, data_dictionary::databa
     values.reserve(c.elements.size());
     bool all_terminal = true;
     for (auto& e : c.elements) {
-        expression elem = prepare_expression(e, db, keyspace, value_spec);
+        expression elem = prepare_expression(e, db, keyspace, schema_opt, value_spec);
 
         if (!is<constant>(elem)) {
             all_terminal = false;
@@ -462,21 +514,39 @@ tuple_constructor_test_assignment(const tuple_constructor& tc, data_dictionary::
 }
 
 static
-expression
-tuple_constructor_prepare_nontuple(const tuple_constructor& tc, data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver) {
-    tuple_constructor_validate_assignable_to(tc, db, keyspace, *receiver);
+std::optional<expression>
+tuple_constructor_prepare_nontuple(const tuple_constructor& tc, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver) {
+    if (receiver) {
+        tuple_constructor_validate_assignable_to(tc, db, keyspace, *receiver);
+    }
     std::vector<expression> values;
     bool all_terminal = true;
     for (size_t i = 0; i < tc.elements.size(); ++i) {
-        expression value = prepare_expression(tc.elements[i], db, keyspace, component_spec_of(*receiver, i));
+        lw_shared_ptr<column_specification> component_receiver;
+        if (receiver) {
+            component_receiver = component_spec_of(*receiver, i);
+        }
+        std::optional<expression> value_opt = try_prepare_expression(tc.elements[i], db, keyspace, schema_opt, component_receiver);
+        if (!value_opt) {
+            return std::nullopt;
+        }
+        auto& value = *value_opt;
         if (!is<constant>(value)) {
             all_terminal = false;
         }
         values.push_back(std::move(value));
     }
+    data_type type;
+    if (receiver) {
+        type = receiver->type;
+    } else {
+        type = tuple_type_impl::get_instance(boost::copy_range<std::vector<data_type>>(
+                values
+                | boost::adaptors::transformed(type_of)));
+    }
     tuple_constructor value {
         .elements  = std::move(values),
-        .type = static_pointer_cast<const tuple_type_impl>(receiver->type)
+        .type = std::move(type),
     };
     if (all_terminal) {
         return evaluate(value, query_options::DEFAULT);
@@ -595,9 +665,13 @@ untyped_constant_test_assignment(const untyped_constant& uc, data_dictionary::da
 }
 
 static
-constant
+std::optional<expression>
 untyped_constant_prepare_expression(const untyped_constant& uc, data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver)
 {
+    if (!receiver) {
+        // TODO: It is possible to infer the type of a constant by looking at the value and selecting the smallest fit
+        return std::nullopt;
+    }
     if (!is_assignable(untyped_constant_test_assignment(uc, db, keyspace, *receiver))) {
         throw exceptions::invalid_request_exception(format("Invalid {} constant ({}) for \"{}\" of type {}",
             uc.partial_type, uc.raw_text, *receiver->name, receiver->type->as_cql3_type().to_string()));
@@ -633,8 +707,12 @@ null_test_assignment(data_dictionary::database db,
 }
 
 static
-constant
+std::optional<expression>
 null_prepare_expression(data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver) {
+    if (!receiver) {
+        // TODO: It is not possible to infer the type of NULL, but perhaps we can have a matcing null_type that can be cast to anything
+        return std::nullopt;
+    }
     if (!is_assignable(null_test_assignment(db, keyspace, *receiver))) {
         throw exceptions::invalid_request_exception("Invalid null value for counter increment/decrement");
     }
@@ -674,8 +752,12 @@ cast_test_assignment(const cast& c, data_dictionary::database db, const sstring&
 }
 
 static
-expression
-cast_prepare_expression(const cast& c, data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver) {
+std::optional<expression>
+cast_prepare_expression(const cast& c, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver) {
+    if (!receiver) {
+        // TODO: It is possible to infer the type of a cast (it's a given)
+        return std::nullopt;
+    }
     auto type = std::get<shared_ptr<cql3_type::raw>>(c.type);
     if (!is_assignable(test_assignment(c.arg, db, keyspace, *casted_spec_of(c, db, keyspace, *receiver)))) {
         throw exceptions::invalid_request_exception(format("Cannot cast value {} to type {}", c.arg, type));
@@ -683,11 +765,18 @@ cast_prepare_expression(const cast& c, data_dictionary::database db, const sstri
     if (!is_assignable(cast_test_assignment(c, db, keyspace, *receiver))) {
         throw exceptions::invalid_request_exception(format("Cannot assign value {} to {} of type {}", c, receiver->name, receiver->type->as_cql3_type()));
     }
-    return prepare_expression(c.arg, db, keyspace, receiver);
+    return cast{
+        .arg = prepare_expression(c.arg, db, keyspace, schema_opt, receiver),
+        .type = receiver->type,
+    };
 }
 
-expr::expression
-prepare_function_call(const expr::function_call& fc, data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver) {
+std::optional<expression>
+prepare_function_call(const expr::function_call& fc, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver) {
+    if (!receiver) {
+        // TODO: It is possible to infer the type of a function call if there is only one overload, or if all overloads return the same type
+        return std::nullopt;
+    }
     auto&& fun = std::visit(overloaded_functor{
         [] (const shared_ptr<functions::function>& func) {
             return func;
@@ -725,7 +814,7 @@ prepare_function_call(const expr::function_call& fc, data_dictionary::database d
     parameters.reserve(fc.args.size());
     bool all_terminal = true;
     for (size_t i = 0; i < fc.args.size(); ++i) {
-        expr::expression e = prepare_expression(fc.args[i], db, keyspace,
+        expr::expression e = prepare_expression(fc.args[i], db, keyspace, schema_opt,
                                                 functions::functions::make_arg_spec(receiver->ks_name, receiver->cf_name, *scalar_fun, i));
         if (!expr::is<expr::constant>(e)) {
             all_terminal = false;
@@ -775,64 +864,108 @@ test_assignment_function_call(const cql3::expr::function_call& fc, data_dictiona
     }
 }
 
-expression
-prepare_expression(const expression& expr, data_dictionary::database db, const sstring& keyspace, lw_shared_ptr<column_specification> receiver) {
+std::optional<expression>
+try_prepare_expression(const expression& expr, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver) {
     return expr::visit(overloaded_functor{
-        [] (const constant&) -> expression {
+        [] (const constant&) -> std::optional<expression> {
             on_internal_error(expr_logger, "Can't prepare constant_value, it should not appear in parser output");
         },
-        [&] (const binary_operator&) -> expression {
+        [&] (const binary_operator&) -> std::optional<expression> {
             on_internal_error(expr_logger, "binary_operators are not yet reachable via prepare_expression()");
         },
-        [&] (const conjunction&) -> expression {
+        [&] (const conjunction&) -> std::optional<expression> {
             on_internal_error(expr_logger, "conjunctions are not yet reachable via prepare_expression()");
         },
-        [&] (const column_value&) -> expression {
-            on_internal_error(expr_logger, "column_values are not yet reachable via prepare_expression()");
+        [] (const column_value& cv) -> std::optional<expression> {
+            return cv;
         },
-        [&] (const subscript&) -> expression {
-            on_internal_error(expr_logger, "subscripts are not yet reachable via prepare_expression()");
+        [&] (const subscript& sub) -> std::optional<expression> {
+            if (!schema_opt) {
+                throw exceptions::invalid_request_exception("cannot process subscript operation without schema");
+            }
+            auto& schema = *schema_opt;
+
+            auto sub_col_opt = try_prepare_expression(sub.val, db, keyspace, schema_opt, receiver);
+            if (!sub_col_opt) {
+                return std::nullopt;
+            }
+            auto& sub_col = *sub_col_opt;
+            const abstract_type& sub_col_type = type_of(sub_col)->without_reversed();
+
+            auto col_spec = column_specification_of(sub_col);
+            lw_shared_ptr<column_specification> subscript_column_spec;
+            if (sub_col_type.is_map()) {
+                subscript_column_spec = map_key_spec_of(*col_spec);
+            } else if (sub_col_type.is_list()) {
+                subscript_column_spec = list_key_spec_of(*col_spec);
+            } else {
+                throw exceptions::invalid_request_exception(format("Column {} is not a map/list, cannot be subscripted", col_spec->name->text()));
+            }
+
+            return subscript {
+                .val = sub_col,
+                .sub = prepare_expression(sub.sub, db, schema.ks_name(), &schema, std::move(subscript_column_spec))
+            };
         },
-        [&] (const token&) -> expression {
-            on_internal_error(expr_logger, "tokens are not yet reachable via prepare_expression()");
+        [&] (const token& tk) -> std::optional<expression> {
+            if (!schema_opt) {
+                throw exceptions::invalid_request_exception("cannot process token() function without schema");
+            }
+            auto& schema = *schema_opt;
+
+            std::vector<expression> prepared_token_args;
+            prepared_token_args.reserve(tk.args.size());
+
+            for (const expression& arg : tk.args) {
+                auto prepared_arg_opt = try_prepare_expression(arg, db, keyspace, schema_opt, receiver);
+                if (!prepared_arg_opt) {
+                    return std::nullopt;
+                }
+                prepared_token_args.emplace_back(std::move(*prepared_arg_opt));
+            }
+
+            return token(std::move(prepared_token_args));
         },
-        [&] (const unresolved_identifier&) -> expression {
-            on_internal_error(expr_logger, "unresolved_identifiers are not yet reachable via prepare_expression()");
+        [&] (const unresolved_identifier& unin) -> std::optional<expression> {
+            if (!schema_opt) {
+                throw exceptions::invalid_request_exception(fmt::format("Cannot resolve column {} without schema", unin.ident->to_cql_string()));
+            }
+            return resolve_column(unin, *schema_opt);
         },
-        [&] (const column_mutation_attribute&) -> expression {
+        [&] (const column_mutation_attribute&) -> std::optional<expression> {
             on_internal_error(expr_logger, "column_mutation_attributes are not yet reachable via prepare_expression()");
         },
-        [&] (const function_call& fc) -> expression {
-            return prepare_function_call(fc, db, keyspace, std::move(receiver));
+        [&] (const function_call& fc) -> std::optional<expression> {
+            return prepare_function_call(fc, db, keyspace, schema_opt, std::move(receiver));
         },
-        [&] (const cast& c) -> expression {
-            return cast_prepare_expression(c, db, keyspace, receiver);
+        [&] (const cast& c) -> std::optional<expression> {
+            return cast_prepare_expression(c, db, keyspace, schema_opt, receiver);
         },
-        [&] (const field_selection&) -> expression {
+        [&] (const field_selection&) -> std::optional<expression> {
             on_internal_error(expr_logger, "field_selections are not yet reachable via prepare_expression()");
         },
-        [&] (const null&) -> expression {
+        [&] (const null&) -> std::optional<expression> {
             return null_prepare_expression(db, keyspace, receiver);
         },
-        [&] (const bind_variable& bv) -> expression {
+        [&] (const bind_variable& bv) -> std::optional<expression> {
             return bind_variable_prepare_expression(bv, db, keyspace, receiver);
         },
-        [&] (const untyped_constant& uc) -> expression {
+        [&] (const untyped_constant& uc) -> std::optional<expression> {
             return untyped_constant_prepare_expression(uc, db, keyspace, receiver);
         },
-        [&] (const tuple_constructor& tc) -> expression {
-            return tuple_constructor_prepare_nontuple(tc, db, keyspace, receiver);
+        [&] (const tuple_constructor& tc) -> std::optional<expression> {
+            return tuple_constructor_prepare_nontuple(tc, db, keyspace, schema_opt, receiver);
         },
-        [&] (const collection_constructor& c) -> expression {
+        [&] (const collection_constructor& c) -> std::optional<expression> {
             switch (c.style) {
-            case collection_constructor::style_type::list: return list_prepare_expression(c, db, keyspace, receiver);
-            case collection_constructor::style_type::set: return set_prepare_expression(c, db, keyspace, receiver);
-            case collection_constructor::style_type::map: return map_prepare_expression(c, db, keyspace, receiver);
+            case collection_constructor::style_type::list: return list_prepare_expression(c, db, keyspace, schema_opt, receiver);
+            case collection_constructor::style_type::set: return set_prepare_expression(c, db, keyspace, schema_opt, receiver);
+            case collection_constructor::style_type::map: return map_prepare_expression(c, db, keyspace, schema_opt, receiver);
             }
             on_internal_error(expr_logger, fmt::format("unexpected collection_constructor style {}", static_cast<unsigned>(c.style)));
         },
-        [&] (const usertype_constructor& uc) -> expression {
-            return usertype_constructor_prepare_expression(uc, db, keyspace, receiver);
+        [&] (const usertype_constructor& uc) -> std::optional<expression> {
+            return usertype_constructor_prepare_expression(uc, db, keyspace, schema_opt, receiver);
         },
     }, expr);
 }
@@ -901,6 +1034,15 @@ test_assignment(const expression& expr, data_dictionary::database db, const sstr
     }, expr);
 }
 
+expression
+prepare_expression(const expression& expr, data_dictionary::database db, const sstring& keyspace, const schema* schema_opt, lw_shared_ptr<column_specification> receiver) {
+    auto e_opt = try_prepare_expression(expr, db, keyspace, schema_opt, std::move(receiver));
+    if (!e_opt) {
+        throw exceptions::invalid_request_exception(fmt::format("Could not infer type of {}", expr));
+    }
+    return std::move(*e_opt);
+}
+
 assignment_testable::test_result
 test_assignment_all(const std::vector<expression>& to_test, data_dictionary::database db, const sstring& keyspace, const column_specification& receiver) {
     using test_result = assignment_testable::test_result;
@@ -935,82 +1077,15 @@ public:
 }
 
 // Finds column_defintion for given column name in the schema.
-// Second argument is a relation to print in the error message in case of failure.
-static const column_value resolve_column(const unresolved_identifier& col_ident, const schema& schema, const binary_operator& printable_relation) {
+static const column_value resolve_column(const unresolved_identifier& col_ident, const schema& schema) {
     ::shared_ptr<column_identifier> id = col_ident.ident->prepare_column_identifier(schema);
     const column_definition* def = get_column_definition(schema, *id);
     if (!def || def->is_hidden_from_cql()) {
-        throw exceptions::unrecognized_entity_exception(*id, fmt::format("{}", printable_relation));
+        throw exceptions::unrecognized_entity_exception(*id, fmt::format("{}", col_ident));
     }
     return column_value(def);
 }
 
-// Resolves columns on LHS of binary_operator and prepares the index in case of a subscripted column.
-// Last argument is a relation to print in the error message in case of failure.
-static expression prepare_binop_lhs(const expression& lhs, data_dictionary::database db, const schema& schema, const binary_operator& printable_relation) {
-    return expr::visit(overloaded_functor{
-        [&](const unresolved_identifier& unin) -> expression {
-            return resolve_column(unin, schema, printable_relation);
-        },
-        [](const column_value& cv) -> expression { return cv; },
-        [&](const tuple_constructor& tc) -> expression {
-            std::vector<expression> new_elements;
-            new_elements.reserve(tc.elements.size());
-
-            for (const expression& elem : tc.elements) {
-                new_elements.emplace_back(prepare_binop_lhs(elem, db, schema, printable_relation));
-            }
-
-            return tuple_constructor {
-                .elements = std::move(new_elements)
-            };
-        },
-        [&](const token& tk) -> expression {
-            std::vector<expression> prepared_token_args;
-            prepared_token_args.reserve(tk.args.size());
-
-            for (const expression& arg : tk.args) {
-                prepared_token_args.emplace_back(prepare_binop_lhs(arg, db, schema, printable_relation));
-            }
-
-            return token(std::move(prepared_token_args));
-        },
-        [&](const subscript& sub) -> expression {
-            const column_definition* sub_col = expr::visit(overloaded_functor {
-                [&](const unresolved_identifier& unin) -> const column_definition* {
-                    return resolve_column(unin, schema, printable_relation).col;
-                },
-                [](const column_value& cv) -> const column_definition* {
-                    return cv.col;
-                },
-                [](const auto& e) -> const column_definition* {
-                    on_internal_error(expr_logger,
-                        fmt::format("Only columns can be subscripted using the [] operator, got {}", e));
-                }
-            }, sub.val);
-
-            const abstract_type& sub_col_type = sub_col->column_specification->type->without_reversed();
-
-            lw_shared_ptr<column_specification> subscript_column_spec;
-            if (sub_col_type.is_map()) {
-                subscript_column_spec = map_key_spec_of(*sub_col->column_specification);
-            } else if (sub_col_type.is_list()) {
-                subscript_column_spec = list_key_spec_of(*sub_col->column_specification);
-            } else {
-                throw exceptions::invalid_request_exception(format("Column {} is not a map/list, cannot be subscripted", sub_col->name_as_text()));
-            }
-
-            return subscript {
-                .val = column_value(sub_col),
-                .sub = prepare_expression(sub.sub, db, schema.ks_name(), std::move(subscript_column_spec))
-            };
-        },
-        [](const auto& e) -> expression {
-            on_internal_error(expr_logger, format("prepare_binop_lhs: Unhandled expression: {}", e));
-        }
-    }, lhs);
-}
-
 // Finds the type of a prepared LHS of binary_operator and creates a receiver with it.
 static lw_shared_ptr<column_specification> get_lhs_receiver(const expression& prepared_lhs, const schema& schema) {
     return expr::visit(overloaded_functor{
@@ -1100,12 +1175,16 @@ static lw_shared_ptr<column_specification> get_rhs_receiver(lw_shared_ptr<column
     }
 }
 
-binary_operator prepare_binary_operator(const binary_operator& binop, data_dictionary::database db, schema_ptr schema, prepare_context& ctx) {
-    expression prepared_lhs = prepare_binop_lhs(binop.lhs, db, *schema, binop);
+binary_operator prepare_binary_operator(const binary_operator& binop, data_dictionary::database db, schema_ptr schema) {
+    std::optional<expression> prepared_lhs_opt = try_prepare_expression(binop.lhs, db, "", schema.get(), {});
+    if (!prepared_lhs_opt) {
+        throw exceptions::invalid_request_exception(fmt::format("Could not infer type of {}", binop.lhs));
+    }
+    auto& prepared_lhs = *prepared_lhs_opt;
     lw_shared_ptr<column_specification> lhs_receiver = get_lhs_receiver(prepared_lhs, *schema);
 
     lw_shared_ptr<column_specification> rhs_receiver = get_rhs_receiver(lhs_receiver, binop.op);
-    expression prepared_rhs = prepare_expression(binop.rhs, db, schema->ks_name(), rhs_receiver);
+    expression prepared_rhs = prepare_expression(binop.rhs, db, schema->ks_name(), schema.get(), rhs_receiver);
 
     return binary_operator(std::move(prepared_lhs), binop.op, std::move(prepared_rhs), binop.order);
 }

cql3/expr/to_restriction.cc

@@ -334,7 +334,7 @@ void preliminary_binop_vaidation_checks(const binary_operator& binop) {
         }
     }
 
-    binary_operator prepared_binop = prepare_binary_operator(binop_to_prepare, db, schema, ctx);
+    binary_operator prepared_binop = prepare_binary_operator(binop_to_prepare, db, schema);
 
     const column_value* lhs_pk_col_search_res = find_in_expression<column_value>(prepared_binop.lhs,
         [](const column_value& col) {

cql3/operation.cc

@@ -37,19 +37,19 @@ operation::set_element::prepare(data_dictionary::database db, const sstring& key
     }
 
     if (rtype->get_kind() == abstract_type::kind::list) {
-        auto&& lval = prepare_expression(_value, db, keyspace, lists::value_spec_of(*receiver.column_specification));
+        auto&& lval = prepare_expression(_value, db, keyspace, nullptr, lists::value_spec_of(*receiver.column_specification));
         if (_by_uuid) {
-            auto&& idx = prepare_expression(_selector, db, keyspace, lists::uuid_index_spec_of(*receiver.column_specification));
+            auto&& idx = prepare_expression(_selector, db, keyspace, nullptr, lists::uuid_index_spec_of(*receiver.column_specification));
             return make_shared<lists::setter_by_uuid>(receiver, std::move(idx), std::move(lval));
         } else {
-            auto&& idx = prepare_expression(_selector, db, keyspace, lists::index_spec_of(*receiver.column_specification));
+            auto&& idx = prepare_expression(_selector, db, keyspace, nullptr, lists::index_spec_of(*receiver.column_specification));
             return make_shared<lists::setter_by_index>(receiver, std::move(idx), std::move(lval));
         }
     } else if (rtype->get_kind() == abstract_type::kind::set) {
         throw invalid_request_exception(format("Invalid operation ({}) for set column {}", to_string(receiver), receiver.name()));
     } else if (rtype->get_kind() == abstract_type::kind::map) {
-        auto key = prepare_expression(_selector, db, keyspace, maps::key_spec_of(*receiver.column_specification));
-        auto mval = prepare_expression(_value, db, keyspace, maps::value_spec_of(*receiver.column_specification));
+        auto key = prepare_expression(_selector, db, keyspace, nullptr, maps::key_spec_of(*receiver.column_specification));
+        auto mval = prepare_expression(_value, db, keyspace, nullptr, maps::value_spec_of(*receiver.column_specification));
         return make_shared<maps::setter_by_key>(receiver, std::move(key), std::move(mval));
     }
     abort();
@@ -84,7 +84,7 @@ operation::set_field::prepare(data_dictionary::database db, const sstring& keysp
                 format("UDT column {} does not have a field named {}", receiver.name_as_text(), *_field));
     }
 
-    auto val = prepare_expression(_value, db, keyspace, user_types::field_spec_of(*receiver.column_specification, *idx));
+    auto val = prepare_expression(_value, db, keyspace, nullptr, user_types::field_spec_of(*receiver.column_specification, *idx));
     return make_shared<user_types::setter_by_field>(receiver, *idx, std::move(val));
 }
 
@@ -130,7 +130,7 @@ operation::addition::to_string(const column_definition& receiver) const {
 
 shared_ptr<operation>
 operation::addition::prepare(data_dictionary::database db, const sstring& keyspace, const column_definition& receiver) const {
-    auto v = prepare_expression(_value, db, keyspace, receiver.column_specification);
+    auto v = prepare_expression(_value, db, keyspace, nullptr, receiver.column_specification);
 
     auto ctype = dynamic_pointer_cast<const collection_type_impl>(receiver.type);
     if (!ctype) {
@@ -170,7 +170,7 @@ operation::subtraction::prepare(data_dictionary::database db, const sstring& key
         if (!receiver.is_counter()) {
             throw exceptions::invalid_request_exception(format("Invalid operation ({}) for non counter column {}", to_string(receiver), receiver.name()));
         }
-        auto v = prepare_expression(_value, db, keyspace, receiver.column_specification);
+        auto v = prepare_expression(_value, db, keyspace, nullptr, receiver.column_specification);
         return make_shared<constants::subtracter>(receiver, std::move(v));
     }
     if (!ctype->is_multi_cell()) {
@@ -179,9 +179,9 @@ operation::subtraction::prepare(data_dictionary::database db, const sstring& key
     }
 
     if (ctype->get_kind() == abstract_type::kind::list) {
-        return make_shared<lists::discarder>(receiver, prepare_expression(_value, db, keyspace, receiver.column_specification));
+        return make_shared<lists::discarder>(receiver, prepare_expression(_value, db, keyspace, nullptr, receiver.column_specification));
     } else if (ctype->get_kind() == abstract_type::kind::set) {
-        return make_shared<sets::discarder>(receiver, prepare_expression(_value, db, keyspace, receiver.column_specification));
+        return make_shared<sets::discarder>(receiver, prepare_expression(_value, db, keyspace, nullptr, receiver.column_specification));
     } else if (ctype->get_kind() == abstract_type::kind::map) {
         auto&& mtype = dynamic_pointer_cast<const map_type_impl>(ctype);
         // The value for a map subtraction is actually a set
@@ -190,7 +190,7 @@ operation::subtraction::prepare(data_dictionary::database db, const sstring& key
                 receiver.column_specification->cf_name,
                 receiver.column_specification->name,
                 set_type_impl::get_instance(mtype->get_keys_type(), false));
-        return ::make_shared<sets::discarder>(receiver, prepare_expression(_value, db, keyspace, std::move(vr)));
+        return ::make_shared<sets::discarder>(receiver, prepare_expression(_value, db, keyspace, nullptr, std::move(vr)));
     }
     abort();
 }
@@ -207,7 +207,7 @@ operation::prepend::to_string(const column_definition& receiver) const {
 
 shared_ptr<operation>
 operation::prepend::prepare(data_dictionary::database db, const sstring& keyspace, const column_definition& receiver) const {
-    auto v = prepare_expression(_value, db, keyspace, receiver.column_specification);
+    auto v = prepare_expression(_value, db, keyspace, nullptr, receiver.column_specification);
 
     if (!dynamic_cast<const list_type_impl*>(receiver.type.get())) {
         throw exceptions::invalid_request_exception(format("Invalid operation ({}) for non list column {}", to_string(receiver), receiver.name()));
@@ -226,7 +226,7 @@ operation::prepend::is_compatible_with(const std::unique_ptr<raw_update>& other)
 
 ::shared_ptr <operation>
 operation::set_value::prepare(data_dictionary::database db, const sstring& keyspace, const column_definition& receiver) const {
-    auto v = prepare_expression(_value, db, keyspace, receiver.column_specification);
+    auto v = prepare_expression(_value, db, keyspace, nullptr, receiver.column_specification);
 
     if (receiver.type->is_counter()) {
         throw exceptions::invalid_request_exception(format("Cannot set the value of counter column {} (counters can only be incremented/decremented, not set)", receiver.name_as_text()));
@@ -264,7 +264,7 @@ operation::set_counter_value_from_tuple_list::prepare(data_dictionary::database
     // We need to fake a column of list<tuple<...>> to prepare the value expression
     auto & os = receiver.column_specification;
     auto spec = make_lw_shared<cql3::column_specification>(os->ks_name, os->cf_name, os->name, counter_tuple_list_type);
-    auto v = prepare_expression(_value, db, keyspace, spec);
+    auto v = prepare_expression(_value, db, keyspace, nullptr, spec);
 
     // Will not be used elsewhere, so make it local.
     class counter_setter : public operation {
@@ -349,13 +349,13 @@ operation::element_deletion::prepare(data_dictionary::database db, const sstring
     }
     auto ctype = static_pointer_cast<const collection_type_impl>(receiver.type);
     if (ctype->get_kind() == abstract_type::kind::list) {
-        auto&& idx = prepare_expression(_element, db, keyspace, lists::index_spec_of(*receiver.column_specification));
+        auto&& idx = prepare_expression(_element, db, keyspace, nullptr, lists::index_spec_of(*receiver.column_specification));
         return make_shared<lists::discarder_by_index>(receiver, std::move(idx));
     } else if (ctype->get_kind() == abstract_type::kind::set) {
-        auto&& elt = prepare_expression(_element, db, keyspace, sets::value_spec_of(*receiver.column_specification));
+        auto&& elt = prepare_expression(_element, db, keyspace, nullptr, sets::value_spec_of(*receiver.column_specification));
         return make_shared<sets::element_discarder>(receiver, std::move(elt));
     } else if (ctype->get_kind() == abstract_type::kind::map) {
-        auto&& key = prepare_expression(_element, db, keyspace, maps::key_spec_of(*receiver.column_specification));
+        auto&& key = prepare_expression(_element, db, keyspace, nullptr, maps::key_spec_of(*receiver.column_specification));
         return make_shared<maps::discarder_by_key>(receiver, std::move(key));
     }
     abort();

cql3/selection/selectable.cc

@@ -143,14 +143,14 @@ shared_ptr<selector::factory>
 selectable::with_cast::new_selector_factory(data_dictionary::database db, schema_ptr s, std::vector<const column_definition*>& defs) {
     std::vector<shared_ptr<selectable>> args{_arg};
     auto&& factories = selector_factories::create_factories_and_collect_column_definitions(args, db, s, defs);
-    auto&& fun = functions::castas_functions::get(_type.get_type(), factories->new_instances());
+    auto&& fun = functions::castas_functions::get(_type, factories->new_instances());
 
     return abstract_function_selector::new_factory(std::move(fun), std::move(factories));
 }
 
 sstring
 selectable::with_cast::to_string() const {
-    return format("cast({} as {})", _arg->to_string(), _type.to_string());
+    return format("cast({} as {})", _arg->to_string(), cql3_type(_type).to_string());
 }
 
 shared_ptr<selectable>
@@ -209,7 +209,7 @@ prepare_selectable(const schema& s, const expr::expression& raw_selectable) {
             }, fc.func);
         },
         [&] (const expr::cast& c) -> shared_ptr<selectable> {
-            auto t = std::get_if<cql3_type>(&c.type);
+            auto t = std::get_if<data_type>(&c.type);
             if (!t) {
                 // FIXME: adjust prepare_seletable() signature so we can prepare the type too
                 on_internal_error(slogger, "unprepared type in selector type cast");

cql3/selection/selectable.hh

@@ -78,9 +78,9 @@ public:
 
 class selectable::with_cast : public selectable {
     ::shared_ptr<selectable> _arg;
-    cql3_type _type;
+    data_type _type;
 public:
-    with_cast(::shared_ptr<selectable> arg, cql3_type type)
+    with_cast(::shared_ptr<selectable> arg, data_type type)
         : _arg(std::move(arg)), _type(std::move(type)) {
     }
 

cql3/statements/create_aggregate_statement.cc

@@ -54,7 +54,7 @@ shared_ptr<functions::function> create_aggregate_statement::create(query_process
     if (_ival) {
         auto dummy_ident = ::make_shared<column_identifier>("", true);
         auto column_spec = make_lw_shared<column_specification>("", "", dummy_ident, state_type);
-        auto initcond_term = expr::evaluate(prepare_expression(_ival.value(), db, _name.keyspace, {column_spec}), query_options::DEFAULT);
+        auto initcond_term = expr::evaluate(prepare_expression(_ival.value(), db, _name.keyspace, nullptr, {column_spec}), query_options::DEFAULT);
         initcond = std::move(initcond_term.value).to_bytes();
     }
 

cql3/statements/select_statement.cc

@@ -1732,7 +1732,7 @@ select_statement::prepare_limit(data_dictionary::database db, prepare_context& c
         return std::nullopt;
     }
 
-    expr::expression prep_limit = prepare_expression(*limit, db, keyspace(), limit_receiver());
+    expr::expression prep_limit = prepare_expression(*limit, db, keyspace(), nullptr, limit_receiver());
     expr::fill_prepare_context(prep_limit, ctx);
     return prep_limit;
 }

cql3/statements/update_statement.cc

@@ -333,7 +333,7 @@ insert_json_statement::prepare_internal(data_dictionary::database db, schema_ptr
     (void)_if_not_exists;
     assert(expr::is<cql3::expr::untyped_constant>(_json_value) || expr::is<cql3::expr::bind_variable>(_json_value));
     auto json_column_placeholder = ::make_shared<column_identifier>("", true);
-    auto prepared_json_value = prepare_expression(_json_value, db, "", make_lw_shared<column_specification>("", "", json_column_placeholder, utf8_type));
+    auto prepared_json_value = prepare_expression(_json_value, db, "", nullptr, make_lw_shared<column_specification>("", "", json_column_placeholder, utf8_type));
     expr::fill_prepare_context(prepared_json_value, ctx);
     auto stmt = ::make_shared<cql3::statements::insert_prepared_json_statement>(ctx.bound_variables_size(), schema, std::move(attrs), stats, std::move(prepared_json_value), _default_unset);
     prepare_conditions(db, *schema, ctx, *stmt);