cql3: expr, lists: convert lists::literal to new collection_constructor

Introduce a collection_constructor (similar to C++'s std::initializer_list)
to hold subexpressions being gathered into a list. Since sets, maps, and
lists construction share some attributes (all elements must be of the
same type) collection_constructor will be used for all of them, so it
also holds an enum. I used "style" for the enum since it's a weak
attribute - an empty set is also an empty map. I chose collection_constructor
rather than plain 'collection' to highlight that it's not the only way
to get a collection (selecting a collection column is another, as an
example) and to hint at what it does - construct a collection from
more primitive elements.

cql3/Cql.g

@@ -1465,7 +1465,11 @@ collectionLiteral returns [shared_ptr<cql3::term::raw> value]
 	@init{ std::vector<shared_ptr<cql3::term::raw>> l; }
     : '['
           ( t1=term { l.push_back(t1); } ( ',' tn=term { l.push_back(tn); } )* )?
-      ']' { $value = ::make_shared<cql3::lists::literal>(std::move(l)); }
+      ']' { $value = cql3::expr::as_term_raw(
+                        cql3::expr::collection_constructor{
+                            cql3::expr::collection_constructor::style_type::list,
+                            boost::copy_range<std::vector<cql3::expr::expression>>(std::move(l) | boost::adaptors::transformed(cql3::expr::as_expression))
+                        }); }
     | '{' t=term v=setOrMapLiteral[t] { $value = v; } '}'
     // Note that we have an ambiguity between maps and set for "{}". So we force it to a set literal,
     // and deal with it later based on the type of the column (SetLiteral.java).

cql3/expr/expression.cc

@@ -566,6 +566,9 @@ bool is_satisfied_by(const binary_operator& opr, const column_value_eval_bag& ba
             [] (const tuple_constructor&) -> bool {
                 on_internal_error(expr_logger, "is_satisified_by: tuple_constructor cannot serve as the LHS of a binary expression (yet!)");
             },
+            [] (const collection_constructor&) -> bool {
+                on_internal_error(expr_logger, "is_satisified_by: collection_constructor cannot serve as the LHS of a binary expression");
+            },
         }, *opr.lhs);
 }
 
@@ -617,6 +620,9 @@ bool is_satisfied_by(const expression& restr, const column_value_eval_bag& bag)
             [] (const tuple_constructor&) -> bool {
                 on_internal_error(expr_logger, "is_satisfied_by: a tuple constructor cannot serve as a restriction by itself");
             },
+            [] (const collection_constructor&) -> bool {
+                on_internal_error(expr_logger, "is_satisfied_by: a collection constructor cannot serve as a restriction by itself");
+            },
         }, restr);
 }
 
@@ -868,6 +874,9 @@ value_set possible_lhs_values(const column_definition* cdef, const expression& e
                         [] (const tuple_constructor&) -> value_set {
                             on_internal_error(expr_logger, "possible_lhs_values: tuple constructors are not supported as the LHS of a binary expression yet");
                         },
+                        [] (const collection_constructor&) -> value_set {
+                            on_internal_error(expr_logger, "possible_lhs_values: collection constructors are not supported as the LHS of a binary expression");
+                        },
                     }, *oper.lhs);
             },
             [] (const column_value&) -> value_set {
@@ -909,6 +918,9 @@ value_set possible_lhs_values(const column_definition* cdef, const expression& e
             [] (const tuple_constructor&) -> value_set {
                 on_internal_error(expr_logger, "possible_lhs_values: an tuple constructor cannot serve as a restriction by itself");
             },
+            [] (const collection_constructor&) -> value_set {
+                on_internal_error(expr_logger, "possible_lhs_values: a collection constructor cannot serve as a restriction by itself");
+            },
         }, expr);
 }
 
@@ -982,6 +994,9 @@ bool is_supported_by(const expression& expr, const secondary_index::index& idx)
                         [&] (const tuple_constructor&) -> bool {
                             on_internal_error(expr_logger, "is_supported_by: tuple constructors are not supported as the LHS of a binary expression yet");
                         },
+                        [&] (const collection_constructor&) -> bool {
+                            on_internal_error(expr_logger, "is_supported_by: collection constructors are not supported as the LHS of a binary expression");
+                        },
                     }, *oper.lhs);
             },
             [] (const auto& default_case) { return false; }
@@ -1074,6 +1089,12 @@ std::ostream& operator<<(std::ostream& os, const expression& expr) {
             [&] (const tuple_constructor& tc) {
                 fmt::print(os, "({})", join(", ", tc.elements));
             },
+            [&] (const collection_constructor& cc) {
+                switch (cc.style) {
+                case collection_constructor::style_type::list: fmt::print(os, "{}", std::to_string(cc.elements)); return;
+                }
+                on_internal_error(expr_logger, fmt::format("unexpected collection_constructor style {}", static_cast<unsigned>(cc.style)));
+            },
         }, expr);
     return os;
 }
@@ -1128,6 +1149,16 @@ expression replace_column_def(const expression& expr, const column_definition* n
                      }
                  };
             },
+            [&] (const collection_constructor& c) {
+                return expression{
+                    collection_constructor{
+                        c.style,
+                        boost::copy_range<std::vector<expression>>(
+                            c.elements | boost::adaptors::transformed(std::bind(replace_column_def, std::placeholders::_1, new_cdef))
+                        )
+                    }
+                };
+            },
         }, expr);
 }
 
@@ -1186,6 +1217,13 @@ expression replace_token(const expression& expr, const column_definition* new_cd
                     tc.elements | boost::adaptors::transformed(std::bind(replace_token, std::placeholders::_1, new_cdef))
                 )};
             },
+            [&] (const collection_constructor& c) -> expression {
+                return collection_constructor{
+                    c.style,
+                    boost::copy_range<std::vector<expression>>(
+                        c.elements | boost::adaptors::transformed(std::bind(replace_token, std::placeholders::_1, new_cdef)))
+                };
+            },
         }, expr);
 }
 
@@ -1277,6 +1315,7 @@ std::vector<expression> extract_single_column_restrictions_for_column(const expr
         void operator()(const bind_variable&) {}
         void operator()(const untyped_constant&) {}
         void operator()(const tuple_constructor&) {}
+        void operator()(const collection_constructor&) {}
     };
 
     visitor v {

cql3/expr/expression.hh

@@ -86,12 +86,13 @@ struct null;
 struct bind_variable;
 struct untyped_constant;
 struct tuple_constructor;
+struct collection_constructor;
 
 /// A CQL expression -- union of all possible expression types.  bool means a Boolean constant.
 using expression = std::variant<bool, conjunction, binary_operator, column_value, column_value_tuple, token,
                                 unresolved_identifier, column_mutation_attribute, function_call, cast,
                                 field_selection, term_raw_ptr, null, bind_variable, untyped_constant,
-                                tuple_constructor>;
+                                tuple_constructor, collection_constructor>;
 
 template <typename T>
 concept ExpressionElement = utils::VariantElement<T, expression>;
@@ -221,6 +222,13 @@ struct tuple_constructor {
     std::vector<expression> elements;
 };
 
+// Constructs a collection of same-typed elements
+struct collection_constructor {
+    enum class style_type { list };
+    style_type style;
+    std::vector<expression> elements;
+};
+
 /// Creates a conjunction of a and b.  If either a or b is itself a conjunction, its children are inserted
 /// directly into the resulting conjunction's children, flattening the expression tree.
 extern expression make_conjunction(expression a, expression b);
@@ -332,6 +340,14 @@ const binary_operator* find_atom(const expression& e, Fn f) {
                 }
                 return nullptr;
             },
+            [&] (const collection_constructor& c) -> const binary_operator* {
+                for (auto& e : c.elements) {
+                    if (auto found = find_atom(e, f)) {
+                        return found;
+                    }
+                }
+                return nullptr;
+            },
         }, e);
 }
 
@@ -376,6 +392,10 @@ size_t count_if(const expression& e, Fn f) {
                 return std::accumulate(t.elements.cbegin(), t.elements.cend(), size_t{0},
                                        [&] (size_t acc, const expression& e) { return acc + count_if(e, f); });
             },
+            [&] (const collection_constructor& c) -> size_t {
+                return std::accumulate(c.elements.cbegin(), c.elements.cend(), size_t{0},
+                                       [&] (size_t acc, const expression& e) { return acc + count_if(e, f); });
+            },
         }, e);
 }
 

cql3/expr/term_expr.cc

@@ -30,73 +30,72 @@
 #include "cql3/tuples.hh"
 #include "types/list.hh"
 
-namespace cql3 {
-
-sstring
-lists::literal::to_string() const {
-    return std::to_string(_elements);
-}
+namespace cql3::expr {
 
+static
 lw_shared_ptr<column_specification>
-lists::value_spec_of(const column_specification& column) {
+list_value_spec_of(const column_specification& column) {
     return make_lw_shared<column_specification>(column.ks_name, column.cf_name,
             ::make_shared<column_identifier>(format("value({})", *column.name), true),
                 dynamic_cast<const list_type_impl&>(column.type->without_reversed()).get_elements_type());
 }
 
+static
 void
-lists::literal::validate_assignable_to(database& db, const sstring keyspace, const column_specification& receiver) const {
+list_validate_assignable_to(const collection_constructor& c, database& db, const sstring keyspace, const column_specification& receiver) {
     if (!receiver.type->without_reversed().is_list()) {
         throw exceptions::invalid_request_exception(format("Invalid list literal for {} of type {}",
                 *receiver.name, receiver.type->as_cql3_type()));
     }
-    auto&& value_spec = value_spec_of(receiver);
-    for (auto rt : _elements) {
-        if (!is_assignable(rt->test_assignment(db, keyspace, *value_spec))) {
+    auto&& value_spec = list_value_spec_of(receiver);
+    for (auto& rt : c.elements) {
+        if (!is_assignable(as_term_raw(rt)->test_assignment(db, keyspace, *value_spec))) {
             throw exceptions::invalid_request_exception(format("Invalid list literal for {}: value {} is not of type {}",
-                    *receiver.name, *rt, value_spec->type->as_cql3_type()));
+                    *receiver.name, rt, value_spec->type->as_cql3_type()));
         }
     }
 }
 
+static
 assignment_testable::test_result
-lists::literal::test_assignment(database& db, const sstring& keyspace, const column_specification& receiver) const {
+list_test_assignment(const collection_constructor& c, database& db, const sstring& keyspace, const column_specification& receiver) {
     if (!dynamic_pointer_cast<const list_type_impl>(receiver.type)) {
         return assignment_testable::test_result::NOT_ASSIGNABLE;
     }
 
     // If there is no elements, we can't say it's an exact match (an empty list if fundamentally polymorphic).
-    if (_elements.empty()) {
+    if (c.elements.empty()) {
         return assignment_testable::test_result::WEAKLY_ASSIGNABLE;
     }
 
-    auto&& value_spec = value_spec_of(receiver);
+    auto&& value_spec = list_value_spec_of(receiver);
     std::vector<shared_ptr<assignment_testable>> to_test;
-    to_test.reserve(_elements.size());
-    std::copy(_elements.begin(), _elements.end(), std::back_inserter(to_test));
+    to_test.reserve(c.elements.size());
+    boost::copy(c.elements | boost::adaptors::transformed(as_term_raw), std::back_inserter(to_test));
     return assignment_testable::test_all(db, keyspace, *value_spec, to_test);
 }
 
 
+static
 shared_ptr<term>
-lists::literal::prepare(database& db, const sstring& keyspace, const column_specification_or_tuple& receiver_) const {
+list_prepare_term(const collection_constructor& c, database& db, const sstring& keyspace, const column_specification_or_tuple& receiver_) {
     auto receiver = std::get<lw_shared_ptr<column_specification>>(receiver_);
-    validate_assignable_to(db, keyspace, *receiver);
+    list_validate_assignable_to(c, db, keyspace, *receiver);
 
     // In Cassandra, an empty (unfrozen) map/set/list is equivalent to the column being null. In
     // other words a non-frozen collection only exists if it has elements. Return nullptr right
     // away to simplify predicate evaluation. See also
     // https://issues.apache.org/jira/browse/CASSANDRA-5141
-    if (receiver->type->is_multi_cell() &&  _elements.empty()) {
+    if (receiver->type->is_multi_cell() &&  c.elements.empty()) {
         return cql3::constants::NULL_VALUE;
     }
 
-    auto&& value_spec = value_spec_of(*receiver);
+    auto&& value_spec = list_value_spec_of(*receiver);
     std::vector<shared_ptr<term>> values;
-    values.reserve(_elements.size());
+    values.reserve(c.elements.size());
     bool all_terminal = true;
-    for (auto rt : _elements) {
-        auto&& t = rt->prepare(db, keyspace, value_spec);
+    for (auto& rt : c.elements) {
+        auto&& t = as_term_raw(rt)->prepare(db, keyspace, value_spec);
 
         if (t->contains_bind_marker()) {
             throw exceptions::invalid_request_exception(format("Invalid list literal for {}: bind variables are not supported inside collection literals", *receiver->name));
@@ -106,18 +105,14 @@ lists::literal::prepare(database& db, const sstring& keyspace, const column_spec
         }
         values.push_back(std::move(t));
     }
-    delayed_value value(values);
+    lists::delayed_value value(values);
     if (all_terminal) {
         return value.bind(query_options::DEFAULT);
     } else {
-        return make_shared<delayed_value>(std::move(value));
+        return make_shared<lists::delayed_value>(std::move(value));
     }
 }
 
-}
-
-namespace cql3::expr {
-
 static
 lw_shared_ptr<column_specification>
 component_spec_of(const column_specification& column, size_t component) {
@@ -571,6 +566,12 @@ term_raw_expr::prepare(database& db, const sstring& keyspace, const column_speci
         [&] (const tuple_constructor& tc) -> ::shared_ptr<term> {
             return tuple_constructor_prepare_term(tc, db, keyspace, receiver);
         },
+        [&] (const collection_constructor& c) -> ::shared_ptr<term> {
+            switch (c.style) {
+            case collection_constructor::style_type::list: return list_prepare_term(c, db, keyspace, receiver);
+            }
+            on_internal_error(expr_logger, fmt::format("unexpected collection_constructor style {}", static_cast<unsigned>(c.style)));
+        },
     }, _expr);
 }
 
@@ -626,6 +627,12 @@ term_raw_expr::test_assignment(database& db, const sstring& keyspace, const colu
         [&] (const tuple_constructor& tc) -> test_result {
             return tuple_constructor_test_assignment(tc, db, keyspace, receiver);
         },
+        [&] (const collection_constructor& c) -> test_result {
+            switch (c.style) {
+            case collection_constructor::style_type::list: return list_test_assignment(c, db, keyspace, receiver);
+            }
+            on_internal_error(expr_logger, fmt::format("unexpected collection_constructor style {}", static_cast<unsigned>(c.style)));
+        },
     }, _expr);
 }
 
@@ -650,4 +657,13 @@ term_raw_expr::assignment_testable_source_context() const {
 }
 
 
-}
+}
+
+namespace cql3 {
+
+lw_shared_ptr<column_specification>
+lists::value_spec_of(const column_specification& column) {
+    return cql3::expr::list_value_spec_of(column);
+}
+
+}

cql3/lists.hh

@@ -58,20 +58,6 @@ public:
     static lw_shared_ptr<column_specification> value_spec_of(const column_specification&);
     static lw_shared_ptr<column_specification> uuid_index_spec_of(const column_specification&);
 
-    class literal : public term::raw {
-        const std::vector<shared_ptr<term::raw>> _elements;
-    public:
-        explicit literal(std::vector<shared_ptr<term::raw>> elements)
-            : _elements(std::move(elements)) {
-        }
-        virtual shared_ptr<term> prepare(database& db, const sstring& keyspace, const column_specification_or_tuple& receiver) const override;
-    private:
-        void validate_assignable_to(database& db, const sstring keyspace, const column_specification& receiver) const;
-    public:
-        virtual assignment_testable::test_result test_assignment(database& db, const sstring& keyspace, const column_specification& receiver) const override;
-        virtual sstring to_string() const override;
-    };
-
     class value : public multi_item_terminal, collection_terminal {
     public:
         utils::chunked_vector<managed_bytes_opt> _elements;

cql3/restrictions/statement_restrictions.cc

@@ -246,6 +246,10 @@ static std::vector<expr::expression> extract_partition_range(
         void operator()(const tuple_constructor&) {
             on_internal_error(rlogger, "extract_partition_range(tuple_constructor)");
         }
+
+        void operator()(const collection_constructor&) {
+            on_internal_error(rlogger, "extract_partition_range(collection_constructor)");
+        }
     } v;
     std::visit(v, where_clause);
     if (v.tokens) {
@@ -351,6 +355,10 @@ static std::vector<expr::expression> extract_clustering_prefix_restrictions(
         void operator()(const tuple_constructor&) {
             on_internal_error(rlogger, "extract_clustering_prefix_restrictions(tuple_constructor)");
         }
+
+        void operator()(const collection_constructor&) {
+            on_internal_error(rlogger, "extract_clustering_prefix_restrictions(collection_constructor)");
+        }
     } v;
     std::visit(v, where_clause);
 
@@ -1068,6 +1076,10 @@ struct multi_column_range_accumulator {
         on_internal_error(rlogger, "tuple constructor encountered outside binary operator");
     }
 
+    void operator()(const collection_constructor&) {
+        on_internal_error(rlogger, "collection constructor encountered outside binary operator");
+    }
+
     /// Intersects each range with v.  If any intersection is empty, clears ranges.
     void intersect_all(const query::clustering_range& v) {
         for (auto& r : ranges) {

cql3/selection/selectable.cc

@@ -223,6 +223,9 @@ prepare_selectable(const schema& s, const expr::expression& raw_selectable) {
         [&] (const expr::tuple_constructor&) -> shared_ptr<selectable> {
             on_internal_error(slogger, "tuple_constructor found its way to selector context");
         },
+        [&] (const expr::collection_constructor&) -> shared_ptr<selectable> {
+            on_internal_error(slogger, "collection_constructor found its way to selector context");
+        },
     }, raw_selectable);
 }
 
@@ -281,6 +284,9 @@ selectable_processes_selection(const expr::expression& raw_selectable) {
         [&] (const expr::tuple_constructor&) -> bool {
             on_internal_error(slogger, "tuple_constructor found its way to selector context");
         },
+        [&] (const expr::collection_constructor&) -> bool {
+            on_internal_error(slogger, "collection_constructor found its way to selector context");
+        },
     }, raw_selectable);
 };