Skip to content

Commit

Permalink
Fix issue with syncing a Decimal128 with big significand (#5740)
Browse files Browse the repository at this point in the history
* Will transfer all bits to server
* Update Baas docker image version
  • Loading branch information
jedelbo authored Sep 1, 2022
1 parent 2a3911c commit f640818
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* Fix all UBSan failures hit by tests. It is unclear if any of these manifested as visible bugs. ([PR #5665](https://github.com/realm/realm-core/pull/5665))
* Fix sorting order for `realm_query_find_first` in the C API.([#5720](https://github.com/realm/realm-core/issues/5720))
* Upload completion callbacks may have called before the download message that completed them was fully integrated. ([#4865](https://github.com/realm/realm-core/issues/4865)).
* Syncing of a Decimal128 with big significand could result in a crash. ([#5728](https://github.com/realm/realm-core/issues/5728))


### Breaking changes
Expand Down
2 changes: 1 addition & 1 deletion dependencies.list
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
PACKAGE_NAME=realm-core
VERSION=12.5.1
OPENSSL_VERSION=1.1.1n
MDBREALM_TEST_SERVER_TAG=2022-08-10
MDBREALM_TEST_SERVER_TAG=2022-09-01
2 changes: 1 addition & 1 deletion evergreen/install_baas.sh
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ case $(uname -s) in
DISTRO_VERSION_MAJOR="$(cut -d. -f1 <<< "$DISTRO_VERSION" )"
fi
case $DISTRO_NAME in
ubuntu)
ubuntu | linuxmint)
MONGODB_DOWNLOAD_URL="http://downloads.10gen.com/linux/mongodb-linux-$(uname -m)-enterprise-ubuntu${DISTRO_VERSION_MAJOR}04-5.0.3.tgz"
STITCH_ASSISTED_AGG_LIB_URL="https://stitch-artifacts.s3.amazonaws.com/stitch-mongo-libs/stitch_mongo_libs_ubuntu2004_x86_64_0bdbed3d42ea136e166b3aad8f6fd09f336b1668_22_03_29_14_36_02/libmongo-ubuntu2004-x86_64.so"
STITCH_SUPPORT_LIB_URL="https://mciuploads.s3.amazonaws.com/mongodb-mongo-v4.4/stitch-support/ubuntu2004/58971da1ef93435a9f62bf4708a81713def6e88c/stitch-support-4.4.9-73-g58971da.tgz"
Expand Down
4 changes: 3 additions & 1 deletion src/realm/sync/changeset_encoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -389,10 +389,12 @@ void ChangesetEncoder::append_value(Decimal128 id)
int exp;
bool sign;
id.unpack(cx, exp, sign);
char buffer[16];
constexpr int max_bytes = 17; // 113 bits / 7
char buffer[max_bytes];
_impl::Bid128 tmp;
memcpy(&tmp, &cx, sizeof(Decimal128::Bid128));
auto n = _impl::encode_int(buffer, tmp);
REALM_ASSERT(n <= max_bytes);
append_bytes(buffer, n);
append_value(int64_t(exp));
append_value(sign);
Expand Down
2 changes: 1 addition & 1 deletion src/realm/sync/noinst/integer_codec.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ bool decode_int(I& input, _impl::Bid128& value) noexcept
{
uint64_t value_0 = 0;
uint64_t value_1 = 0;
constexpr int max_bytes = 16; // 112 bits / 7
constexpr int max_bytes = 17; // 113 bits / 7

int i = 0;
for (;;) {
Expand Down
49 changes: 49 additions & 0 deletions test/object-store/sync/app.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1586,6 +1586,55 @@ TEST_CASE("app: mixed lists with object links", "[sync][app]") {
}
}

TEST_CASE("app: roundtrip values", "[sync][app]") {
std::string base_url = get_base_url();
const std::string valid_pk_name = "_id";
REQUIRE(!base_url.empty());

Schema schema{
{"TopLevel",
{
{valid_pk_name, PropertyType::ObjectId, Property::IsPrimary{true}},
{"decimal", PropertyType::Decimal | PropertyType::Nullable},
}},
};

auto server_app_config = minimal_app_config(base_url, "roundtrip_values", schema);
auto app_session = create_app(server_app_config);
auto partition = random_string(100);

Decimal128 large_significand = Decimal128(70) / Decimal128(1.09);
auto obj_id = ObjectId::gen();
{
TestAppSession test_session(app_session, nullptr, DeleteApp{false});
SyncTestFile config(test_session.app(), partition, schema);
auto realm = Realm::get_shared_realm(config);

CppContext c(realm);
realm->begin_transaction();
Object::create(c, realm, "TopLevel",
util::Any(AnyDict{
{valid_pk_name, obj_id},
{"decimal", large_significand},
}),
CreatePolicy::ForceCreate);
realm->commit_transaction();
CHECK(!wait_for_upload(*realm, std::chrono::seconds(600)));
}

{
TestAppSession test_session(app_session);
SyncTestFile config(test_session.app(), partition, schema);
auto realm = Realm::get_shared_realm(config);

CHECK(!wait_for_download(*realm));
CppContext c(realm);
auto obj = Object::get_for_primary_key(c, realm, "TopLevel", util::Any{obj_id});
auto val = obj.get_column_value<Decimal128>("decimal");
CHECK(val == large_significand);
}
}

TEST_CASE("app: upgrade from local to synced realm", "[sync][app]") {
std::string base_url = get_base_url();
const std::string valid_pk_name = "_id";
Expand Down
7 changes: 6 additions & 1 deletion test/test_instruction_replication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,17 +215,20 @@ TEST(InstructionReplication_CreateObjectObjectIdPK)
Decimal128 large_w0_zero(uint64_t(0x1234500000000000));
large_w0_zero *= Decimal128(0x1000000);
large_w0_zero += Decimal128(0x23);
Decimal128 large_significand = Decimal128(70) / Decimal128(1.09);
Fixture fixture{test_context};
ObjKey key;
ObjKey key2;
ObjKey key3;
ObjKey key4;
{
WriteTransaction wt{fixture.sg_1};
TableRef foo = wt.get_group().add_table_with_primary_key("class_foo", type_ObjectId, "_id", false);
auto col_dec = foo->add_column(type_Decimal, "cost");
key = foo->create_object_with_primary_key(id).set(col_dec, cost).get_key();
key2 = foo->create_object_with_primary_key(ObjectId::gen()).set(col_dec, large).get_key();
key3 = foo->create_object_with_primary_key(ObjectId::gen()).set(col_dec, large_w0_zero).get_key();
key4 = foo->create_object_with_primary_key(ObjectId::gen()).set(col_dec, large_significand).get_key();
wt.commit();
}
fixture.replay_transactions();
Expand All @@ -234,7 +237,7 @@ TEST(InstructionReplication_CreateObjectObjectIdPK)
ReadTransaction rt{fixture.sg_2};
CHECK(rt.has_table("class_foo"));
ConstTableRef foo = rt.get_table("class_foo");
CHECK_EQUAL(foo->size(), 3);
CHECK_EQUAL(foo->size(), 4);
ColKey col_ndx = foo->get_column_key("_id");
ColKey col_dec = foo->get_column_key("cost");
auto obj = foo->get_object(key);
Expand All @@ -244,6 +247,8 @@ TEST(InstructionReplication_CreateObjectObjectIdPK)
CHECK_EQUAL(obj.get<Decimal128>(col_dec), large);
obj = foo->get_object(key3);
CHECK_EQUAL(obj.get<Decimal128>(col_dec), large_w0_zero);
obj = foo->get_object(key4);
CHECK_EQUAL(obj.get<Decimal128>(col_dec), large_significand);
}
}

Expand Down

0 comments on commit f640818

Please sign in to comment.