You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by is...@apache.org on 2023/10/05 09:07:22 UTC
[ignite-3] branch main updated: IGNITE-20240 C++ 3.0: Reject Tuples with unmapped fields (#2653)
This is an automated email from the ASF dual-hosted git repository.
isapego pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new b0aabdf31d IGNITE-20240 C++ 3.0: Reject Tuples with unmapped fields (#2653)
b0aabdf31d is described below
commit b0aabdf31d10790e76a1976e391b1515777e1846
Author: Igor Sapego <is...@apache.org>
AuthorDate: Thu Oct 5 13:07:14 2023 +0400
IGNITE-20240 C++ 3.0: Reject Tuples with unmapped fields (#2653)
---
.../platforms/cpp/ignite/client/detail/utils.cpp | 37 +++++-
.../client-test/key_value_binary_view_test.cpp | 18 +++
.../tests/client-test/record_binary_view_test.cpp | 19 ++-
.../cpp/tests/client-test/record_view_test.cpp | 129 ++++++++++++++-------
4 files changed, 157 insertions(+), 46 deletions(-)
diff --git a/modules/platforms/cpp/ignite/client/detail/utils.cpp b/modules/platforms/cpp/ignite/client/detail/utils.cpp
index 5a7e06111e..e4bddbbe91 100644
--- a/modules/platforms/cpp/ignite/client/detail/utils.cpp
+++ b/modules/platforms/cpp/ignite/client/detail/utils.cpp
@@ -201,19 +201,46 @@ std::vector<std::byte> pack_tuple(
builder.claim_null();
}
+ std::int32_t written = 0;
builder.layout();
for (std::int32_t i = 0; i < count; ++i) {
const auto &col = sch.columns[i];
auto col_idx = tuple.column_ordinal(col.name);
- if (col_idx >= 0)
+ if (col_idx >= 0) {
append_column(builder, col.type, tuple.get(col_idx), col.scale);
- else {
+ ++written;
+ } else {
builder.append_null();
no_value.set(std::size_t(i));
}
}
+ if (!key_only && written < tuple.column_count()) {
+ std::vector<bool> written_ind(tuple.column_count(), false);
+ for (std::int32_t i = 0; i < count; ++i) {
+ const auto &col = sch.columns[i];
+ auto col_idx = tuple.column_ordinal(col.name);
+
+ if (col_idx >= 0)
+ written_ind[col_idx] = true;
+ }
+
+ std::stringstream unmapped_columns;
+ for (std::int32_t i = 0; i < tuple.column_count(); ++i) {
+ if (written_ind[i])
+ continue;
+ auto &name = tuple.column_name(i);
+ unmapped_columns << name << ",";
+ }
+ auto unmapped_columns_str = unmapped_columns.str();
+ unmapped_columns_str.pop_back();
+
+ assert(!unmapped_columns_str.empty());
+ throw ignite_error("Key tuple doesn't match schema: schemaVersion="
+ + std::to_string(sch.version) + ", extraColumns=" + unmapped_columns_str);
+ }
+
return builder.build();
}
@@ -225,8 +252,10 @@ ignite_tuple concat(const ignite_tuple &left, const ignite_tuple &right) {
res.m_indices.insert(left.m_indices.begin(), left.m_indices.end());
for (const auto &pair : right.m_pairs) {
- res.m_pairs.emplace_back(pair);
- res.m_indices.emplace(ignite_tuple::parse_name(pair.first), res.m_pairs.size() - 1);
+ bool inserted = res.m_indices.emplace(ignite_tuple::parse_name(pair.first), res.m_pairs.size()).second;
+ if (inserted) {
+ res.m_pairs.emplace_back(pair);
+ }
}
return res;
diff --git a/modules/platforms/cpp/tests/client-test/key_value_binary_view_test.cpp b/modules/platforms/cpp/tests/client-test/key_value_binary_view_test.cpp
index 38fc94e035..e218333686 100644
--- a/modules/platforms/cpp/tests/client-test/key_value_binary_view_test.cpp
+++ b/modules/platforms/cpp/tests/client-test/key_value_binary_view_test.cpp
@@ -22,6 +22,7 @@
#include "ignite/client/ignite_client_configuration.h"
#include <gtest/gtest.h>
+#include <gmock/gmock-matchers.h>
#include <chrono>
@@ -139,6 +140,23 @@ TEST_F(key_value_binary_view_test, put_empty_value_throws) {
ignite_error);
}
+TEST_F(key_value_binary_view_test, put_extra_collumn_value_throws) {
+ auto key_tuple = get_tuple(1);
+ auto val_tuple = get_tuple("foo");
+ val_tuple.set("extra", std::string("extra"));
+ EXPECT_THROW(
+ {
+ try {
+ kv_view.put(nullptr, key_tuple, val_tuple);
+ } catch (const ignite_error &e) {
+ EXPECT_THAT(e.what_str(), testing::MatchesRegex(
+ "Key tuple doesn't match schema: schemaVersion=.+, extraColumns=extra"));
+ throw;
+ }
+ },
+ ignite_error);
+}
+
TEST_F(key_value_binary_view_test, get_empty_tuple_throws) {
EXPECT_THROW(
{
diff --git a/modules/platforms/cpp/tests/client-test/record_binary_view_test.cpp b/modules/platforms/cpp/tests/client-test/record_binary_view_test.cpp
index b77e7e7425..6ade8c51f0 100644
--- a/modules/platforms/cpp/tests/client-test/record_binary_view_test.cpp
+++ b/modules/platforms/cpp/tests/client-test/record_binary_view_test.cpp
@@ -22,6 +22,7 @@
#include "ignite/client/ignite_client_configuration.h"
#include <gtest/gtest.h>
+#include <gmock/gmock-matchers.h>
#include <chrono>
@@ -126,6 +127,22 @@ TEST_F(record_binary_view_test, upsert_empty_tuple_throws) {
ignite_error);
}
+TEST_F(record_binary_view_test, upsert_tuple_with_extra_columns_throws) {
+ auto val_tuple = get_tuple(1, "foo");
+ val_tuple.set("extra", std::string("some value"));
+ EXPECT_THROW(
+ {
+ try {
+ tuple_view.upsert(nullptr, val_tuple);
+ } catch (const ignite_error &e) {
+ EXPECT_THAT(e.what_str(), testing::MatchesRegex(
+ "Key tuple doesn't match schema: schemaVersion=.+, extraColumns=extra"));
+ throw;
+ }
+ },
+ ignite_error);
+}
+
TEST_F(record_binary_view_test, get_empty_tuple_throws) {
EXPECT_THROW(
{
@@ -781,7 +798,7 @@ TEST_F(record_binary_view_test, remove_exact_empty_throws) {
}
TEST_F(record_binary_view_test, get_and_remove_nonexisting) {
- auto res = tuple_view.get_and_remove(nullptr, get_tuple(42, "foo"));
+ auto res = tuple_view.get_and_remove(nullptr, get_tuple(42));
ASSERT_FALSE(res.has_value());
auto res_tuple = tuple_view.get(nullptr, get_tuple(42));
diff --git a/modules/platforms/cpp/tests/client-test/record_view_test.cpp b/modules/platforms/cpp/tests/client-test/record_view_test.cpp
index 73935a1331..d3cd03d560 100644
--- a/modules/platforms/cpp/tests/client-test/record_view_test.cpp
+++ b/modules/platforms/cpp/tests/client-test/record_view_test.cpp
@@ -50,16 +50,16 @@ struct test_type {
std::string val;
};
-struct wrong_mapping_key {
- wrong_mapping_key() = default;
+struct missing_mapping_key {
+ missing_mapping_key() = default;
- explicit wrong_mapping_key(std::int64_t key)
+ explicit missing_mapping_key(std::int64_t key)
: key(key) {}
- explicit wrong_mapping_key(std::string val)
+ explicit missing_mapping_key(std::string val)
: val(std::move(val)) {}
- explicit wrong_mapping_key(std::int64_t key, std::string val)
+ explicit missing_mapping_key(std::int64_t key, std::string val)
: key(key)
, val(std::move(val)) {}
@@ -67,16 +67,16 @@ struct wrong_mapping_key {
std::string val;
};
-struct wrong_mapping_value {
- wrong_mapping_value() = default;
+struct missing_mapping_value {
+ missing_mapping_value() = default;
- explicit wrong_mapping_value(std::int64_t key)
+ explicit missing_mapping_value(std::int64_t key)
: key(key) {}
- explicit wrong_mapping_value(std::string val)
+ explicit missing_mapping_value(std::string val)
: val(std::move(val)) {}
- explicit wrong_mapping_value(std::int64_t key, std::string val)
+ explicit missing_mapping_value(std::int64_t key, std::string val)
: key(key)
, val(std::move(val)) {}
@@ -84,6 +84,24 @@ struct wrong_mapping_value {
std::string val;
};
+struct extra_mapping_value {
+ extra_mapping_value() = default;
+
+ explicit extra_mapping_value(std::int64_t key)
+ : key(key) {}
+
+ explicit extra_mapping_value(std::string val)
+ : val(std::move(val)) {}
+
+ explicit extra_mapping_value(std::int64_t key, std::string val)
+ : key(key)
+ , val(std::move(val)) {}
+
+ std::int64_t key{0};
+ std::string val;
+};
+
+
namespace ignite {
template<>
@@ -109,45 +127,62 @@ test_type convert_from_tuple(ignite_tuple &&value) {
}
template<>
-ignite_tuple convert_to_tuple(wrong_mapping_key &&value) {
+ignite_tuple convert_to_tuple(missing_mapping_key &&value) {
ignite_tuple tuple;
- tuple.set("id", value.key);
tuple.set("val", value.val);
return tuple;
}
template<>
-wrong_mapping_key convert_from_tuple(ignite_tuple &&value) {
- wrong_mapping_key res;
+missing_mapping_key convert_from_tuple(ignite_tuple &&value) {
+ missing_mapping_key res;
- res.key = value.get<std::int64_t>("id");
+ res.val = value.get<std::string>("val");
- if (value.column_count() > 1)
- res.val = value.get<std::string>("val");
+ return res;
+}
+
+template<>
+ignite_tuple convert_to_tuple(missing_mapping_value &&value) {
+ ignite_tuple tuple;
+
+ tuple.set("key", value.key);
+
+ return tuple;
+}
+
+template<>
+missing_mapping_value convert_from_tuple(ignite_tuple &&value) {
+ missing_mapping_value res;
+
+ res.key = value.get<std::int64_t>("key");
return res;
}
template<>
-ignite_tuple convert_to_tuple(wrong_mapping_value &&value) {
+ignite_tuple convert_to_tuple(extra_mapping_value &&value) {
ignite_tuple tuple;
tuple.set("key", value.key);
- tuple.set("str", value.val);
+ tuple.set("val", value.val);
+ tuple.set("extra", std::string("extra"));
return tuple;
}
template<>
-wrong_mapping_value convert_from_tuple(ignite_tuple &&value) {
- wrong_mapping_value res;
+extra_mapping_value convert_from_tuple(ignite_tuple &&value) {
+ extra_mapping_value res;
res.key = value.get<std::int64_t>("key");
- if (value.column_count() > 1)
- res.val = value.get<std::string>("str");
+ if (value.column_count() > 1) {
+ res.val = value.get<std::string>("val");
+ (void) value.get<std::string>("extra");
+ }
return res;
}
@@ -185,17 +220,17 @@ protected:
record_view<test_type> view;
};
-TEST_F(record_view_test, wrong_mapped_key_throws) {
+TEST_F(record_view_test, missing_mapping_key_throws) {
{
test_type val{1, "foo"};
view.upsert(nullptr, val);
}
auto table = m_client.get_tables().get_table(TABLE_1);
- auto wrong_view = table->get_record_view<wrong_mapping_key>();
+ auto wrong_view = table->get_record_view<missing_mapping_key>();
- wrong_mapping_key key{1};
- wrong_mapping_key val{1, "foo"};
+ missing_mapping_key key{1};
+ missing_mapping_key val{1, "foo"};
EXPECT_THROW(
{
@@ -220,33 +255,45 @@ TEST_F(record_view_test, wrong_mapped_key_throws) {
ignite_error);
}
-TEST_F(record_view_test, wrong_mapped_value_throws) {
- {
- test_type val{1, "foo"};
- view.upsert(nullptr, val);
- }
-
+TEST_F(record_view_test, extra_mapping_value_throws) {
auto table = m_client.get_tables().get_table(TABLE_1);
- auto wrong_view = table->get_record_view<wrong_mapping_value>();
+ auto wrong_view = table->get_record_view<extra_mapping_value>();
- wrong_mapping_value key{1};
- wrong_mapping_value val{1, "foo"};
-
- // Should work just fine. Having additional field and not providing value field is not an error.
- wrong_view.upsert(nullptr, val);
+ extra_mapping_value key{1};
+ extra_mapping_value val{1, "foo"};
EXPECT_THROW(
{
try {
- (void) wrong_view.get(nullptr, key);
+ wrong_view.upsert(nullptr, val);
} catch (const ignite_error &e) {
- EXPECT_EQ(e.what_str(), "Can not find column with the name 'str' in the tuple");
+ EXPECT_THAT(e.what_str(), testing::MatchesRegex(
+ "Key tuple doesn't match schema: schemaVersion=.+, extraColumns=extra"));
throw;
}
},
ignite_error);
}
+TEST_F(record_view_test, missing_mapped_value_dont_throw) {
+ {
+ test_type val{1, "foo"};
+ view.upsert(nullptr, val);
+ }
+
+ auto table = m_client.get_tables().get_table(TABLE_1);
+ auto wrong_view = table->get_record_view<missing_mapping_value>();
+
+ missing_mapping_value key{1};
+ missing_mapping_value val{1, "foo"};
+
+ // Should work just fine. Having additional field and not providing value field is not an error.
+ wrong_view.upsert(nullptr, val);
+
+ auto actual = wrong_view.get(nullptr, key);
+ EXPECT_TRUE(actual->val.empty());
+}
+
TEST_F(record_view_test, upsert_get) {
test_type key{1};
test_type val{1, "foo"};