cql3: expr: fold null into untyped_constant/constant

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
2026-05-12 19:02:12 +00:00 · 2022-11-29 10:27:41 +02:00
parent 8bc0af9e34
commit ea901fdb9d
9 changed files with 41 additions and 92 deletions
--- a/cql3/Cql.g
+++ b/cql3/Cql.g
@@ -1513,7 +1513,7 @@ value returns [expression value]
    | l=collectionLiteral  { $value = std::move(l); }
    | u=usertypeLiteral    { $value = std::move(u); }
    | t=tupleLiteral       { $value = std::move(t); }
-    | K_NULL               { $value = null(); }
+    | K_NULL               { $value = make_untyped_null(); }
    | e=marker             { $value = std::move(e); }
    ;

@@ -1678,7 +1678,7 @@ relation returns [expression e]
    | K_TOKEN l=tupleOfIdentifiers type=relationType t=term
        { $e = binary_operator(token{std::move(l.elements)}, type, std::move(t)); }
    | name=cident K_IS K_NOT K_NULL {
-          $e = binary_operator(unresolved_identifier{std::move(name)}, oper_t::IS_NOT, null()); }
+          $e = binary_operator(unresolved_identifier{std::move(name)}, oper_t::IS_NOT, make_untyped_null()); }
    | name=cident K_IN marker1=marker
        { $e = binary_operator(unresolved_identifier{std::move(name)}, oper_t::IN, std::move(marker1)); }
    | name=cident K_IN in_values=singleColumnInValues
--- a/cql3/column_condition.cc
+++ b/cql3/column_condition.cc
@@ -309,7 +309,7 @@ column_condition::raw::prepare(data_dictionary::database db, const sstring& keys

    if (_op == expr::oper_t::LIKE) {
        auto literal_term = expr::as_if<expr::untyped_constant>(&*_value);
-        if (literal_term) {
+        if (literal_term && literal_term->partial_type != expr::untyped_constant::type_class::null) {
            // Pass matcher object
            const sstring& pattern = literal_term->raw_text;
            return column_condition::condition(receiver, std::move(collection_element_expression),
--- a/cql3/expr/expression.cc
+++ b/cql3/expr/expression.cc
@@ -607,9 +607,6 @@ bool is_satisfied_by(const expression& restr, const evaluation_inputs& inputs) {
            [] (const field_selection&) -> bool {
                on_internal_error(expr_logger, "is_satisfied_by: a field selection cannot serve as a restriction by itself");
            },
-            [] (const null&) -> bool {
-                on_internal_error(expr_logger, "is_satisfied_by: NULL cannot serve as a restriction by itself");
-            },
            [] (const bind_variable&) -> bool {
                on_internal_error(expr_logger, "is_satisfied_by: a bind variable cannot serve as a restriction by itself");
            },
@@ -698,6 +695,14 @@ expression make_conjunction(expression a, expression b) {
    return conjunction{std::move(children)};
 }

+untyped_constant
+make_untyped_null() {
+    return {
+        .partial_type = untyped_constant::type_class::null,
+        .raw_text = "null",
+    };
+}
+
 std::vector<expression>
 boolean_factors(expression e) {
    std::vector<expression> ret;
@@ -888,9 +893,6 @@ value_set possible_lhs_values(const column_definition* cdef, const expression& e
                        [] (const field_selection&) -> value_set {
                            on_internal_error(expr_logger, "possible_lhs_values: field selections are not supported as the LHS of a binary expression");
                        },
-                        [] (const null&) -> value_set {
-                            on_internal_error(expr_logger, "possible_lhs_values: nulls are not supported as the LHS of a binary expression");
-                        },
                        [] (const bind_variable&) -> value_set {
                            on_internal_error(expr_logger, "possible_lhs_values: bind variables are not supported as the LHS of a binary expression");
                        },
@@ -929,9 +931,6 @@ value_set possible_lhs_values(const column_definition* cdef, const expression& e
            [] (const field_selection&) -> value_set {
                on_internal_error(expr_logger, "possible_lhs_values: a field selection cannot serve as a restriction by itself");
            },
-            [] (const null&) -> value_set {
-                on_internal_error(expr_logger, "possible_lhs_values: a NULL cannot serve as a restriction by itself");
-            },
            [] (const bind_variable&) -> value_set {
                on_internal_error(expr_logger, "possible_lhs_values: a bind variable cannot serve as a restriction by itself");
            },
@@ -1026,9 +1025,6 @@ secondary_index::index::supports_expression_v is_supported_by_helper(const expre
                        [&] (const field_selection&) -> ret_t {
                            on_internal_error(expr_logger, "is_supported_by: field selections are not supported as the LHS of a binary expression");
                        },
-                        [&] (const null&) -> ret_t {
-                            on_internal_error(expr_logger, "is_supported_by: nulls are not supported as the LHS of a binary expression");
-                        },
                        [&] (const bind_variable&) -> ret_t {
                            on_internal_error(expr_logger, "is_supported_by: bind variables are not supported as the LHS of a binary expression");
                        },
@@ -1189,10 +1185,6 @@ std::ostream& operator<<(std::ostream& os, const expression::printer& pr) {
            [&] (const field_selection& fs)  {
                fmt::print(os, "({}.{})", to_printer(fs.structure), fs.field);
            },
-            [&] (const null&) {
-                // FIXME: adjust tests and change to NULL
-                fmt::print(os, "null");
-            },
            [&] (const bind_variable&) {
                // FIXME: store and present bind variable name
                fmt::print(os, "?");
@@ -1599,7 +1591,6 @@ std::vector<expression> extract_single_column_restrictions_for_column(const expr
        void operator()(const function_call&) {}
        void operator()(const cast&) {}
        void operator()(const field_selection&) {}
-        void operator()(const null&) {}
        void operator()(const bind_variable&) {}
        void operator()(const untyped_constant&) {}
        void operator()(const tuple_constructor&) {}
@@ -1758,7 +1749,6 @@ cql3::raw_value evaluate(const expression& e, const evaluation_inputs& inputs) {
            on_internal_error(expr_logger, "Can't evaluate a untyped_constant ");
        },

-        [](const null&) { return cql3::raw_value::make_null(); },
        [](const constant& c) { return c.value; },
        [&](const bind_variable& bind_var) { return evaluate(bind_var, inputs); },
        [&](const tuple_constructor& tup) { return evaluate(tup, inputs); },
@@ -2316,7 +2306,6 @@ void fill_prepare_context(expression& e, prepare_context& ctx) {
            fill_prepare_context(s.sub, ctx);
        },
        [](untyped_constant&) {},
-        [](null&) {},
        [](constant&) {},
    }, e);
 }
--- a/cql3/expr/expression.hh
+++ b/cql3/expr/expression.hh
@@ -76,7 +76,6 @@ struct column_mutation_attribute;
 struct function_call;
 struct cast;
 struct field_selection;
-struct null;
 struct bind_variable;
 struct untyped_constant;
 struct constant;
@@ -96,7 +95,6 @@ concept ExpressionElement
        || std::same_as<T, function_call>
        || std::same_as<T, cast>
        || std::same_as<T, field_selection>
-        || std::same_as<T, null>
        || std::same_as<T, bind_variable>
        || std::same_as<T, untyped_constant>
        || std::same_as<T, constant>
@@ -117,7 +115,6 @@ concept invocable_on_expression
        && std::invocable<Func, function_call>
        && std::invocable<Func, cast>
        && std::invocable<Func, field_selection>
-        && std::invocable<Func, null>
        && std::invocable<Func, bind_variable>
        && std::invocable<Func, untyped_constant>
        && std::invocable<Func, constant>
@@ -138,7 +135,6 @@ concept invocable_on_expression_ref
        && std::invocable<Func, function_call&>
        && std::invocable<Func, cast&>
        && std::invocable<Func, field_selection&>
-        && std::invocable<Func, null&>
        && std::invocable<Func, bind_variable&>
        && std::invocable<Func, untyped_constant&>
        && std::invocable<Func, constant&>
@@ -199,7 +195,6 @@ bool operator==(const expression& e1, const expression& e2);
 template <typename E>
 concept LeafExpression
        = std::same_as<unresolved_identifier, E>
-        || std::same_as<null, E> 
        || std::same_as<bind_variable, E> 
        || std::same_as<untyped_constant, E> 
        || std::same_as<constant, E>
@@ -345,12 +340,6 @@ struct field_selection {
    friend bool operator==(const field_selection&, const field_selection&) = default;
 };

-struct null {
-    data_type type; // may be null before prepare
-
-    friend bool operator==(const null&, const null&) = default;
-};
-
 struct bind_variable {
    int32_t bind_index;

@@ -364,13 +353,15 @@ struct bind_variable {
 // A constant which does not yet have a date type. It is partially typed
 // (we know if it's floating or int) but not sized.
 struct untyped_constant {
-    enum type_class { integer, floating_point, string, boolean, duration, uuid, hex };
+    enum type_class { integer, floating_point, string, boolean, duration, uuid, hex, null };
    type_class partial_type;
    sstring raw_text;

    friend bool operator==(const untyped_constant&, const untyped_constant&) = default;
 };

+untyped_constant make_untyped_null();
+
 // Represents a constant value with known value and type
 // For null and unset the type can sometimes be set to empty_type
 struct constant {
@@ -435,7 +426,7 @@ struct usertype_constructor {
 struct expression::impl final {
    using variant_type = std::variant<
            conjunction, binary_operator, column_value, token, unresolved_identifier,
-            column_mutation_attribute, function_call, cast, field_selection, null,
+            column_mutation_attribute, function_call, cast, field_selection,
            bind_variable, untyped_constant, constant, tuple_constructor, collection_constructor,
            usertype_constructor, subscript>;
    variant_type v;
--- a/cql3/expr/prepare_expr.cc
+++ b/cql3/expr/prepare_expr.cc
@@ -123,7 +123,7 @@ usertype_constructor_prepare_expression(const usertype_constructor& u, data_dict
        auto iraw = u.elements.find(field);
        expression raw;
        if (iraw == u.elements.end()) {
-            raw = expr::null();
+            raw = expr::make_untyped_null();
        } else {
            raw = iraw->second;
            ++found_values;
@@ -582,6 +582,7 @@ operator<<(std::ostream&out, untyped_constant::type_class t)
        case untyped_constant::type_class::boolean:  return out << "BOOLEAN";
        case untyped_constant::type_class::hex:      return out << "HEX";
        case untyped_constant::type_class::duration: return out << "DURATION";
+        case untyped_constant::type_class::null:     return out << "NULL";
    }
    abort();
 }
@@ -609,8 +610,9 @@ static
 assignment_testable::test_result
 untyped_constant_test_assignment(const untyped_constant& uc, data_dictionary::database db, const sstring& keyspace, const column_specification& receiver)
 {
+    bool uc_is_null = uc.partial_type == untyped_constant::type_class::null;
    auto receiver_type = receiver.type->as_cql3_type();
-    if (receiver_type.is_collection() || receiver_type.is_user_type()) {
+    if ((receiver_type.is_collection() || receiver_type.is_user_type()) && !uc_is_null) {
        return assignment_testable::test_result::NOT_ASSIGNABLE;
    }
    if (!receiver_type.is_native()) {
@@ -675,6 +677,10 @@ untyped_constant_test_assignment(const untyped_constant& uc, data_dictionary::da
                return assignment_testable::test_result::EXACT_MATCH;
            }
            break;
+        case untyped_constant::type_class::null:
+            return receiver.type->is_counter()
+                ? assignment_testable::test_result::NOT_ASSIGNABLE
+                : assignment_testable::test_result::WEAKLY_ASSIGNABLE;
    }
    return assignment_testable::test_result::NOT_ASSIGNABLE;
 }
@@ -688,9 +694,18 @@ untyped_constant_prepare_expression(const untyped_constant& uc, data_dictionary:
        return std::nullopt;
    }
    if (!is_assignable(untyped_constant_test_assignment(uc, db, keyspace, *receiver))) {
+      if (uc.partial_type != untyped_constant::type_class::null) {
        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()));
+      } else {
+        throw exceptions::invalid_request_exception("Invalid null value for counter increment/decrement");
+      }
    }
+
+    if (uc.partial_type == untyped_constant::type_class::null) {
+        return constant::make_null(receiver->type);
+    }
+
    raw_value raw_val = cql3::raw_value::make_value(untyped_constant_parsed_value(uc, receiver->type));
    return constant(std::move(raw_val), receiver->type);
 }
@@ -715,29 +730,6 @@ bind_variable_prepare_expression(const bind_variable& bv, data_dictionary::datab
    };
 }

-static
-assignment_testable::test_result
-null_test_assignment(data_dictionary::database db,
-        const sstring& keyspace,
-        const column_specification& receiver) {
-    return receiver.type->is_counter()
-        ? assignment_testable::test_result::NOT_ASSIGNABLE
-        : assignment_testable::test_result::WEAKLY_ASSIGNABLE;
-}
-
-static
-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");
-    }
-    return constant::make_null(receiver->type);
-}
-
 static
 sstring
 cast_display_name(const cast& c) {
@@ -964,9 +956,6 @@ try_prepare_expression(const expression& expr, data_dictionary::database db, con
        [&] (const field_selection&) -> std::optional<expression> {
            on_internal_error(expr_logger, "field_selections are not yet reachable via prepare_expression()");
        },
-        [&] (const null&) -> std::optional<expression> {
-            return null_prepare_expression(db, keyspace, receiver);
-        },
        [&] (const bind_variable& bv) -> std::optional<expression> {
            return bind_variable_prepare_expression(bv, db, keyspace, receiver);
        },
@@ -1028,9 +1017,6 @@ test_assignment(const expression& expr, data_dictionary::database db, const sstr
        [&] (const field_selection&) -> test_result {
            on_internal_error(expr_logger, "field_selections are not yet reachable via test_assignment()");
        },
-        [&] (const null&) -> test_result {
-            return null_test_assignment(db, keyspace, receiver);
-        },
        [&] (const bind_variable& bv) -> test_result {
            return bind_variable_test_assignment(bv, db, keyspace, receiver);
        },
--- a/cql3/expr/restrictions.cc
+++ b/cql3/expr/restrictions.cc
@@ -144,7 +144,7 @@ void preliminary_binop_vaidation_checks(const binary_operator& binop) {
    }

    if (binop.op == oper_t::IS_NOT) {
-        bool rhs_is_null = is<null>(binop.rhs)
+        bool rhs_is_null = (is<untyped_constant>(binop.rhs) && as<untyped_constant>(binop.rhs).partial_type == untyped_constant::type_class::null)
                           || (is<constant>(binop.rhs) && as<constant>(binop.rhs).is_null());
        if (!rhs_is_null) {
            throw exceptions::invalid_request_exception(format("Unsupported \"IS NOT\" relation: {}", pretty_binop_printer));
--- a/cql3/restrictions/statement_restrictions.cc
+++ b/cql3/restrictions/statement_restrictions.cc
@@ -153,10 +153,6 @@ static std::vector<expr::expression> extract_partition_range(
            on_internal_error(rlogger, "extract_partition_range(field_selection)");
        }

-        void operator()(const null&) {
-            on_internal_error(rlogger, "extract_partition_range(null)");
-        }
-
        void operator()(const bind_variable&) {
            on_internal_error(rlogger, "extract_partition_range(bind_variable)");
        }
@@ -278,10 +274,6 @@ static std::vector<expr::expression> extract_clustering_prefix_restrictions(
            on_internal_error(rlogger, "extract_clustering_prefix_restrictions(field_selection)");
        }

-        void operator()(const null&) {
-            on_internal_error(rlogger, "extract_clustering_prefix_restrictions(null)");
-        }
-
        void operator()(const bind_variable&) {
            on_internal_error(rlogger, "extract_clustering_prefix_restrictions(bind_variable)");
        }
@@ -1239,10 +1231,6 @@ struct multi_column_range_accumulator {
        on_internal_error(rlogger, "field selection encountered outside binary operator");
    }

-    void operator()(const null&) {
-        on_internal_error(rlogger, "null encountered outside binary operator");
-    }
-
    void operator()(const bind_variable&) {
        on_internal_error(rlogger, "bind variable encountered outside binary operator");
    }
--- a/cql3/selection/selectable.cc
+++ b/cql3/selection/selectable.cc
@@ -222,9 +222,6 @@ prepare_selectable(const schema& s, const expr::expression& raw_selectable) {
            return make_shared<selectable::with_field_selection>(prepare_selectable(s, fs.structure),
                    fs.field->prepare(s));
        },
-        [&] (const expr::null&) -> shared_ptr<selectable> {
-            on_internal_error(slogger, "null found its way to selector context");
-        },
        [&] (const expr::bind_variable&) -> shared_ptr<selectable> {
            on_internal_error(slogger, "bind_variable found its way to selector context");
        },
@@ -283,9 +280,6 @@ selectable_processes_selection(const expr::expression& raw_selectable) {
        [&] (const expr::field_selection& fs) -> bool {
            return true;
        },
-        [&] (const expr::null&) -> bool {
-            on_internal_error(slogger, "null found its way to selector context");
-        },
        [&] (const expr::bind_variable&) -> bool {
            on_internal_error(slogger, "bind_variable found its way to selector context");
        },
--- a/test/boost/expr_test.cc
+++ b/test/boost/expr_test.cc
@@ -1529,7 +1529,7 @@ BOOST_AUTO_TEST_CASE(prepare_null) {
    schema_ptr table_schema = make_simple_test_schema();
    auto [db, db_data] = make_data_dictionary_database(table_schema);

-    expression null_expr = null{};
+    expression null_expr = make_untyped_null();

    expression prepared = prepare_expression(null_expr, db, "test_ks", table_schema.get(), make_receiver(int32_type));
    expression expected = constant::make_null(int32_type);
@@ -1541,7 +1541,7 @@ BOOST_AUTO_TEST_CASE(prepare_null_no_type_fails) {
    schema_ptr table_schema = make_simple_test_schema();
    auto [db, db_data] = make_data_dictionary_database(table_schema);

-    expression null_expr = null{};
+    expression null_expr = make_untyped_null();
    BOOST_REQUIRE_THROW(prepare_expression(null_expr, db, "test_ks", table_schema.get(), nullptr),
                        exceptions::invalid_request_exception);
 }
@@ -1863,7 +1863,7 @@ BOOST_AUTO_TEST_CASE(prepare_list_collection_constructor_with_null) {
        .style = collection_constructor::style_type::list,
        .elements = {untyped_constant{.partial_type = untyped_constant::type_class::integer, .raw_text = "123"},
                     untyped_constant{.partial_type = untyped_constant::type_class::integer, .raw_text = "456"},
-                     null{}},
+                     make_untyped_null()},
        .type = nullptr};

    data_type list_type = list_type_impl::get_instance(long_type, true);
@@ -1992,7 +1992,7 @@ BOOST_AUTO_TEST_CASE(prepare_set_collection_constructor_with_null) {
        .elements =
            {
                untyped_constant{.partial_type = untyped_constant::type_class::integer, .raw_text = "789"},
-                null{},
+                make_untyped_null(),
                untyped_constant{.partial_type = untyped_constant::type_class::integer, .raw_text = "456"},
            },
        .type = nullptr};
@@ -2250,7 +2250,8 @@ BOOST_AUTO_TEST_CASE(prepare_map_collection_constructor_null_key) {
                             untyped_constant{.partial_type = untyped_constant::type_class::integer, .raw_text = "30"}},
                        .type = nullptr},
                    tuple_constructor{
-                        .elements = {null{}, untyped_constant{.partial_type = untyped_constant::type_class::integer,
+                        .elements = {make_untyped_null(),
+                                     untyped_constant{.partial_type = untyped_constant::type_class::integer,
                                                              .raw_text = "-20"}},
                        .type = nullptr},
                    tuple_constructor{
@@ -2283,7 +2284,7 @@ BOOST_AUTO_TEST_CASE(prepare_map_collection_constructor_null_value) {
                                  .type = nullptr},
                tuple_constructor{.elements = {untyped_constant{.partial_type = untyped_constant::type_class::integer,
                                                                .raw_text = "2"},
-                                               null{}},
+                                               make_untyped_null()},
                                  .type = nullptr},
                tuple_constructor{.elements = {untyped_constant{.partial_type = untyped_constant::type_class::integer,
                                                                .raw_text = "1"},
@@ -2358,7 +2359,7 @@ BOOST_AUTO_TEST_CASE(prepare_usertype_constructor_with_null) {
    constructor_elements.emplace(
        column_identifier("field1", true),
        untyped_constant{.partial_type = untyped_constant::type_class::integer, .raw_text = "152"});
-    constructor_elements.emplace(column_identifier("field2", true), null{});
+    constructor_elements.emplace(column_identifier("field2", true), make_untyped_null());
    constructor_elements.emplace(
        column_identifier("field3", true),
        untyped_constant{.partial_type = untyped_constant::type_class::string, .raw_text = "ututu"});