From 83b18597d964bc21c0bb5206f29da4b0a97ada0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Fri, 14 Aug 2015 14:01:54 +0200 Subject: [PATCH 1/6] keys: allow incomplete keys in from_clustering_prefix() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- keys.hh | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/keys.hh b/keys.hh index c9ea327fe5..4921911f7e 100644 --- a/keys.hh +++ b/keys.hh @@ -509,8 +509,15 @@ public: } static clustering_key from_clustering_prefix(const schema& s, const exploded_clustering_prefix& prefix) { - assert(prefix.is_full(s)); - return from_exploded(s, prefix.components()); + if (prefix.is_full(s)) { + return from_exploded(s, prefix.components()); + } + assert(s.is_dense()); + auto components = prefix.components(); + while (components.size() < s.clustering_key_size()) { + components.emplace_back(bytes()); + } + return from_exploded(s, std::move(components)); } friend std::ostream& operator<<(std::ostream& out, const clustering_key& ck); From eeb26ca8de048d099dd34998692fc1b3bdc16599 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Fri, 14 Aug 2015 14:02:57 +0200 Subject: [PATCH 2/6] compound: fix iterator comparison for null values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _v.begin() points to the next element. If the size of last element in a compound is zero then iterators pointing to second to last and last element would seem equal. To fix this we also have to compare _types_left. Signed-off-by: Paweł Dziepak --- compound.hh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compound.hh b/compound.hh index 045b6c516b..837f098bb8 100644 --- a/compound.hh +++ b/compound.hh @@ -177,6 +177,7 @@ public: } else { if (_v.empty()) { if (AllowPrefixes == allow_prefixes::yes) { + _types_left = 0; _v = bytes_view(nullptr, 0); return; } else { @@ -196,7 +197,7 @@ public: iterator(const compound_type& t, const bytes_view& v) : _types_left(t._types.size()), _v(v) { read_current(); } - iterator(end_iterator_tag, const bytes_view& v) : _v(nullptr, 0) {} + iterator(end_iterator_tag, const bytes_view& v) : _types_left(0), _v(nullptr, 0) {} iterator& operator++() { read_current(); return *this; @@ -208,8 +209,8 @@ public: } const value_type& operator*() const { return _current; } const value_type* operator->() const { return &_current; } - bool operator!=(const iterator& i) const { return _v.begin() != i._v.begin(); } - bool operator==(const iterator& i) const { return _v.begin() == i._v.begin(); } + bool operator!=(const iterator& i) const { return _v.begin() != i._v.begin() || _types_left != i._types_left; } + bool operator==(const iterator& i) const { return _v.begin() == i._v.begin() && _types_left == i._types_left; } }; iterator begin(const bytes_view& v) const { return iterator(*this, v); From f6a93be655bf1451b6a86cea447b095d35271063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Fri, 14 Aug 2015 14:06:35 +0200 Subject: [PATCH 3/6] cql3: skip compact value columns with no name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- cql3/selection/selection.cc | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cql3/selection/selection.cc b/cql3/selection/selection.cc index 97833c672d..f065dd36c9 100644 --- a/cql3/selection/selection.cc +++ b/cql3/selection/selection.cc @@ -185,7 +185,15 @@ protected: }; ::shared_ptr selection::wildcard(schema_ptr schema) { - return simple_selection::make(schema, column_definition::vectorize(schema->all_columns_in_select_order()), true); + std::vector cds; + auto& columns = schema->all_columns_in_select_order(); + cds.reserve(columns.size()); + for (auto& c : columns) { + if (!c.is_compact_value() || !c.name().empty()) { + cds.emplace_back(&c); + } + } + return simple_selection::make(schema, std::move(cds), true); } ::shared_ptr selection::for_columns(schema_ptr schema, std::vector columns) { From 111914195930c6ffadd87b8ba91f013802be23e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Fri, 14 Aug 2015 14:07:08 +0200 Subject: [PATCH 4/6] cql3: translate validation code for compact storage tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- cql3/statements/create_table_statement.cc | 50 ++++++++++++----------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/cql3/statements/create_table_statement.cc b/cql3/statements/create_table_statement.cc index b067d81cfc..c18618f3c4 100644 --- a/cql3/statements/create_table_statement.cc +++ b/cql3/statements/create_table_statement.cc @@ -26,6 +26,8 @@ #include #include +#include + #include "cql3/statements/create_table_statement.hh" #include "schema_builder.hh" @@ -256,21 +258,20 @@ create_table_statement::raw_statement::raw_statement(::shared_ptr name, } } -#if 0 - if (!staticColumns.isEmpty()) - { + if (!_static_columns.empty()) { // Only CQL3 tables can have static columns - if (useCompactStorage) - throw new InvalidRequestException("Static columns are not supported in COMPACT STORAGE tables"); + if (_use_compact_storage) { + throw exceptions::invalid_request_exception("Static columns are not supported in COMPACT STORAGE tables"); + } // Static columns only make sense if we have at least one clustering column. Otherwise everything is static anyway - if (columnAliases.isEmpty()) - throw new InvalidRequestException("Static columns are only useful (and thus allowed) if the table has at least one clustering column"); + if (_column_aliases.empty()) { + throw exceptions::invalid_request_exception("Static columns are only useful (and thus allowed) if the table has at least one clustering column"); + } } - if (useCompactStorage && !stmt.columnAliases.isEmpty()) - { - if (stmt.columns.isEmpty()) - { + if (_use_compact_storage && !stmt->_column_aliases.empty()) { + if (stmt->_columns.empty()) { +#if 0 // The only value we'll insert will be the empty one, so the default validator don't matter stmt.defaultValidator = BytesType.instance; // We need to distinguish between @@ -278,32 +279,33 @@ create_table_statement::raw_statement::raw_statement(::shared_ptr name, // * I've defined my table with only a PK (and the column value will be empty) // So, we use an empty valueAlias (rather than null) for the second case stmt.valueAlias = ByteBufferUtil.EMPTY_BYTE_BUFFER; - } - else - { - if (stmt.columns.size() > 1) - throw new InvalidRequestException(String.format("COMPACT STORAGE with composite PRIMARY KEY allows no more than one column not part of the PRIMARY KEY (got: %s)", StringUtils.join(stmt.columns.keySet(), ", "))); - +#endif + } else { + if (stmt->_columns.size() > 1) { + throw exceptions::invalid_request_exception(sprint("COMPACT STORAGE with composite PRIMARY KEY allows no more than one column not part of the PRIMARY KEY (got: %s)", + ::join( ", ", stmt->_columns | boost::adaptors::map_keys))); + } +#if 0 Map.Entry lastEntry = stmt.columns.entrySet().iterator().next(); stmt.defaultValidator = lastEntry.getValue(); stmt.valueAlias = lastEntry.getKey().bytes; stmt.columns.remove(lastEntry.getKey()); +#endif } - } - else - { + } else { // For compact, we are in the "static" case, so we need at least one column defined. For non-compact however, having // just the PK is fine since we have CQL3 row marker. - if (useCompactStorage && stmt.columns.isEmpty()) - throw new InvalidRequestException("COMPACT STORAGE with non-composite PRIMARY KEY require one column not part of the PRIMARY KEY, none given"); - + if (_use_compact_storage && stmt->_columns.empty()) { + throw exceptions::invalid_request_exception("COMPACT STORAGE with non-composite PRIMARY KEY require one column not part of the PRIMARY KEY, none given"); + } +#if 0 // There is no way to insert/access a column that is not defined for non-compact storage, so // the actual validator don't matter much (except that we want to recognize counter CF as limitation apply to them). stmt.defaultValidator = !stmt.columns.isEmpty() && (stmt.columns.values().iterator().next() instanceof CounterColumnType) ? CounterColumnType.instance : BytesType.instance; - } #endif + } // If we give a clustering order, we must explicitly do so for all aliases and in the order of the PK if (!_defined_ordering.empty()) { From b42a9d6e6ac75e69c956e80e7a5df5cfd395e362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Fri, 14 Aug 2015 14:08:21 +0200 Subject: [PATCH 5/6] cql3: translate support of update statements for dense tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- cql3/statements/update_statement.cc | 35 ++++++++++++----------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/cql3/statements/update_statement.cc b/cql3/statements/update_statement.cc index cfefbb857b..85848c7d8c 100644 --- a/cql3/statements/update_statement.cc +++ b/cql3/statements/update_statement.cc @@ -33,29 +33,22 @@ namespace statements { void update_statement::add_update_for_key(mutation& m, const exploded_clustering_prefix& prefix, const update_parameters& params) { if (s->is_dense()) { - throw std::runtime_error("Dense tables not supported yet"); -#if 0 - if (prefix.isEmpty()) - throw new InvalidRequestException(String.format("Missing PRIMARY KEY part %s", cfm.clusteringColumns().get(0))); + if (!prefix) { + throw exceptions::invalid_request_exception(sprint("Missing PRIMARY KEY part %s", *s->clustering_key_columns().begin())); + } - // An empty name for the compact value is what we use to recognize the case where there is not column - // outside the PK, see CreateStatement. - if (!cfm.compactValueColumn().name.bytes.hasRemaining()) - { - // There is no column outside the PK. So no operation could have passed through validation - assert updates.isEmpty(); - new Constants.Setter(cfm.compactValueColumn(), EMPTY).execute(key, cf, prefix, params); + // An empty name for the compact value is what we use to recognize the case where there is not column + // outside the PK, see CreateStatement. + if (s->compact_column().name().empty()) { + // There is no column outside the PK. So no operation could have passed through validation + assert(_column_operations.empty()); + constants::setter(s->compact_column(), make_shared(constants::value(bytes()))).execute(m, prefix, params); + } else { + // dense means we don't have a row marker, so don't accept to set only the PK. See CASSANDRA-5648. + if (_column_operations.empty()) { + throw exceptions::invalid_request_exception(sprint("Column %s is mandatory for this COMPACT STORAGE table", s->compact_column().name_as_text())); } - else - { - // dense means we don't have a row marker, so don't accept to set only the PK. See CASSANDRA-5648. - if (updates.isEmpty()) - throw new InvalidRequestException(String.format("Column %s is mandatory for this COMPACT STORAGE table", cfm.compactValueColumn().name)); - - for (Operation update : updates) - update.execute(key, cf, prefix, params); - } -#endif + } } else { // If there are static columns, there also must be clustering columns, in which // case empty prefix can only refer to the static row. From d9f20ebbd11d2b6ded9db76a9edb1adc35f42917 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Dziepak?= Date: Fri, 14 Aug 2015 14:09:30 +0200 Subject: [PATCH 6/6] tests/cql3: add tests for compact storage tables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Dziepak --- tests/cql_query_test.cc | 66 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/cql_query_test.cc b/tests/cql_query_test.cc index 9ddf43165e..1ee0f6cf2b 100644 --- a/tests/cql_query_test.cc +++ b/tests/cql_query_test.cc @@ -1789,3 +1789,69 @@ SEASTAR_TEST_CASE(test_in_restriction) { }); }); } + +SEASTAR_TEST_CASE(test_compact_storage) { + return do_with_cql_env([] (auto& e) { + return e.execute_cql("create table tcs (p1 int, c1 int, r1 int, PRIMARY KEY (p1, c1)) with compact storage;").discard_result().then([&e] { + return e.require_table_exists("ks", "tcs"); + }).then([&e] { + return e.execute_cql("insert into tcs (p1, c1, r1) values (1, 2, 3);").discard_result(); + }).then([&e] { + return e.require_column_has_value("tcs", {1}, {2}, "r1", 3); + }).then([&e] { + return e.execute_cql("update tcs set r1 = 4 where p1 = 1 and c1 = 2;").discard_result(); + }).then([&e] { + return e.require_column_has_value("tcs", {1}, {2}, "r1", 4); + }).then([&e] { + return e.execute_cql("select * from tcs where p1 = 1;"); + }).then([&e] (auto msg) { + assert_that(msg).is_rows().with_rows({ + { int32_type->decompose(1), int32_type->decompose(2), int32_type->decompose(4) }, + }); + return e.execute_cql("create table tcs2 (p1 int, c1 int, PRIMARY KEY (p1, c1)) with compact storage;").discard_result(); + }).then([&e] { + return e.require_table_exists("ks", "tcs2"); + }).then([&e] { + return e.execute_cql("insert into tcs2 (p1, c1) values (1, 2);").discard_result(); + }).then([&e] { + return e.execute_cql("select * from tcs2 where p1 = 1;"); + }).then([&e] (auto msg) { + assert_that(msg).is_rows().with_rows({ + { int32_type->decompose(1), int32_type->decompose(2) }, + }); + return e.execute_cql("create table tcs3 (p1 int, c1 int, c2 int, r1 int, PRIMARY KEY (p1, c1, c2)) with compact storage;").discard_result(); + }).then([&e] { + return e.execute_cql("insert into tcs3 (p1, c1, c2, r1) values (1, 2, 3, 4);").discard_result(); + }).then([&e] { + return e.execute_cql("insert into tcs3 (p1, c1, r1) values (1, 2, 5);").discard_result(); + }).then([&e] { + return e.execute_cql("insert into tcs3 (p1, c1, r1) values (1, 3, 6);").discard_result(); + }).then([&e] { + return e.execute_cql("insert into tcs3 (p1, c1, c2, r1) values (1, 3, 5, 7);").discard_result(); + }).then([&e] { + return e.execute_cql("select * from tcs3 where p1 = 1;"); + }).then([&e] (auto msg) { + assert_that(msg).is_rows().with_rows({ + { int32_type->decompose(1), int32_type->decompose(2), bytes(), int32_type->decompose(5) }, + { int32_type->decompose(1), int32_type->decompose(2), int32_type->decompose(3), int32_type->decompose(4) }, + { int32_type->decompose(1), int32_type->decompose(3), bytes(), int32_type->decompose(6) }, + { int32_type->decompose(1), int32_type->decompose(3), int32_type->decompose(5), int32_type->decompose(7) }, + }); + return e.execute_cql("delete from tcs3 where p1 = 1 and c1 = 2;").discard_result(); + }).then([&e] { + return e.execute_cql("select * from tcs3 where p1 = 1;"); + }).then([&e] (auto msg) { + assert_that(msg).is_rows().with_rows({ + { int32_type->decompose(1), int32_type->decompose(3), bytes(), int32_type->decompose(6) }, + { int32_type->decompose(1), int32_type->decompose(3), int32_type->decompose(5), int32_type->decompose(7) }, + }); + return e.execute_cql("delete from tcs3 where p1 = 1 and c1 = 3 and c2 = 5;").discard_result(); + }).then([&e] { + return e.execute_cql("select * from tcs3 where p1 = 1;"); + }).then([&e] (auto msg) { + assert_that(msg).is_rows().with_rows({ + { int32_type->decompose(1), int32_type->decompose(3), bytes(), int32_type->decompose(6) }, + }); + }); + }); +}