Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RCORE-2265: Fix issue when changing type of primary key #8046

Merged
merged 2 commits into from
Nov 18, 2024

Conversation

jedelbo
Copy link
Contributor

@jedelbo jedelbo commented Nov 11, 2024

If you change the type of the primary key and call Realm::update_schema() without a migration function, the new column will not be set as primary key column.

What, How & Why?

Fixes #8045

☑️ ToDos

  • 📝 Changelog update
  • 🚦 Tests (or not relevant)
  • C-API, if public C++ API changed
  • bindgen/spec.yml, if public C++ API changed

If you change the type of the primary key and call Realm::update_schema()
without a migration function, the new column will not be set as primary
key column.
Copy link

coveralls-official bot commented Nov 11, 2024

Pull Request Test Coverage Report for Build jorgen.edelbo_455

Details

  • 27 of 27 (100.0%) changed or added relevant lines in 2 files are covered.
  • 70 unchanged lines in 19 files lost coverage.
  • Overall coverage increased (+0.01%) to 91.123%

Files with Coverage Reduction New Missed Lines %
src/realm/dictionary.cpp 1 85.16%
src/realm/query_expression.hpp 1 93.81%
src/realm/util/compression.cpp 1 89.81%
src/realm/uuid.cpp 1 98.48%
src/realm/array_blobs_big.cpp 2 98.58%
src/realm/cluster.cpp 2 75.85%
src/realm/object-store/shared_realm.cpp 2 91.9%
src/realm/sync/noinst/client_impl_base.cpp 3 82.78%
src/realm/sync/noinst/protocol_codec.hpp 3 75.74%
src/realm/table.cpp 3 90.59%
Totals Coverage Status
Change from base Build 2629: 0.01%
Covered Lines: 217269
Relevant Lines: 238434

💛 - Coveralls

@jedelbo jedelbo requested a review from ironage November 14, 2024 08:48
Copy link
Member

@tgoyne tgoyne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth also testing that this works with exactly one object and fails with two objects.


CppContext ctx(realm);
std::any values = AnyDict{
{"_id", UUID("3b241101-0000-0000-0000-4136c566a964"s)},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if the string is not a valid UUID?

Copy link
Contributor Author

@jedelbo jedelbo Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then shit happens. But that is beside the point of this test.

Copy link
Contributor

@ironage ironage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
Could maybe use some more tests on this special case though if you feel inclined to write them.

@jedelbo jedelbo merged commit f7c8e97 into master Nov 18, 2024
40 of 42 checks passed
@jedelbo jedelbo deleted the je/switch-pk-type branch November 18, 2024 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SDK crashes when migrating Primary Key from String to UUID
3 participants