Skip to content

Commit 10572d8

Browse files
fnc12fnc12
authored
Merge pull request #1474 from fnc12/feature/sync-schema-data-loss
added data loss to sync_schema_result
2 parents 6253928 + 4341256 commit 10572d8

7 files changed

Lines changed: 188 additions & 19 deletions

File tree

‎.vscode/tasks.json‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,46 @@
11
{
22
"version": "2.0.0",
33
"tasks": [
4+
{
5+
"label": "CMake Configure (C++17)",
6+
"type": "shell",
7+
"command": "cmake",
8+
"args": [
9+
"-B",
10+
"${workspaceFolder}/build",
11+
"-S",
12+
"${workspaceFolder}",
13+
"-DSQLITE_ORM_ENABLE_CXX_17=ON",
14+
"-DBUILD_TESTING=ON",
15+
"-DCMAKE_BUILD_TYPE=Debug"
16+
],
17+
"group": "build",
18+
"problemMatcher": [],
19+
"presentation": {
20+
"reveal": "always",
21+
"panel": "shared"
22+
}
23+
},
24+
{
25+
"label": "CMake Build (C++17)",
26+
"type": "shell",
27+
"command": "cmake",
28+
"args": [
29+
"--build",
30+
"${workspaceFolder}/build",
31+
"--parallel"
32+
],
33+
"group": {
34+
"kind": "build",
35+
"isDefault": true
36+
},
37+
"dependsOn": "CMake Configure (C++17)",
38+
"problemMatcher": "$gcc",
39+
"presentation": {
40+
"reveal": "always",
41+
"panel": "shared"
42+
}
43+
},
444
{
545
"label": "Run unit_tests",
646
"type": "shell",

‎dev/implementations/storage_definitions.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ namespace sqlite_orm::internal {
110110
this->drop_create_with_loss(db, table);
111111
}
112112
res = schema_stat;
113+
} else if (schema_stat == sync_schema_result::dropped_and_recreated_with_data_loss) {
114+
this->drop_create_with_loss(db, table);
115+
res = schema_stat;
113116
}
114117
}
115118
}

‎dev/storage.h‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,6 +1132,7 @@ namespace sqlite_orm::internal {
11321132

11331133
auto dbTableInfo = this->pragma.table_xinfo(table.name);
11341134
auto res = sync_schema_result::already_in_sync;
1135+
bool canPreserveData = true;
11351136

11361137
// first let's see if table with such name exists..
11371138
auto gottaCreateTable = !this->table_exists(db, table.name);
@@ -1163,7 +1164,20 @@ namespace sqlite_orm::internal {
11631164
}
11641165
}
11651166
if (gottaCreateTable) {
1166-
res = sync_schema_result::dropped_and_recreated;
1167+
// check if any new columns prevent data preservation
1168+
for (const table_xinfo* colInfo: columnsToAdd) {
1169+
if (!table.find_column_generated_storage_type(colInfo->name)) {
1170+
if (colInfo->notnull && colInfo->dflt_value.empty()) {
1171+
canPreserveData = false;
1172+
if (attempt_to_preserve) {
1173+
*attempt_to_preserve = false;
1174+
};
1175+
break;
1176+
}
1177+
}
1178+
}
1179+
res = canPreserveData ? sync_schema_result::dropped_and_recreated
1180+
: sync_schema_result::dropped_and_recreated_with_data_loss;
11671181
} else {
11681182
if (!columnsToAdd.empty()) {
11691183
// extra storage columns than table columns
@@ -1179,6 +1193,7 @@ namespace sqlite_orm::internal {
11791193
} else {
11801194
if (colInfo->notnull && colInfo->dflt_value.empty()) {
11811195
gottaCreateTable = true;
1196+
canPreserveData = false;
11821197
// no matter if preserve is true or false, there is no way to preserve data, so we wont try!
11831198
if (attempt_to_preserve) {
11841199
*attempt_to_preserve = false;
@@ -1194,7 +1209,8 @@ namespace sqlite_orm::internal {
11941209
res = sync_schema_result::new_columns_added;
11951210
}
11961211
} else {
1197-
res = sync_schema_result::dropped_and_recreated;
1212+
res = canPreserveData ? sync_schema_result::dropped_and_recreated
1213+
: sync_schema_result::dropped_and_recreated_with_data_loss;
11981214
}
11991215
} else {
12001216
if (res != sync_schema_result::old_columns_removed) {

‎dev/storage_base.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,11 +1123,11 @@ namespace sqlite_orm::internal {
11231123
(dbColumnInfo.hidden == 0) == (storageColumnInfo.hidden == 0);
11241124
if (!columnsAreEqual) {
11251125
notEqual = true;
1126-
break;
1126+
} else {
1127+
dbTableInfo.erase(dbColumnInfoIt);
1128+
storageTableInfo.erase(storageTableInfo.begin() + storageColumnInfoIndex);
1129+
--storageColumnInfoIndex;
11271130
}
1128-
dbTableInfo.erase(dbColumnInfoIt);
1129-
storageTableInfo.erase(storageTableInfo.begin() + storageColumnInfoIndex);
1130-
--storageColumnInfoIndex;
11311131
} else {
11321132
columnsToAdd.push_back(&storageColumnInfo);
11331133
}

‎dev/sync_schema_result.h‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,18 @@ SQLITE_ORM_EXPORT namespace sqlite_orm {
3636
/**
3737
* old table is dropped and new is recreated. Reasons :
3838
* 1. delete excess columns in the table than storage if preseve = false
39-
* 2. Lacking columns in the table cannot be added due to NULL and DEFAULT constraint
40-
* 3. Reasons 1 and 2 both together
41-
* 4. data_type mismatch between table and storage.
39+
* 2. Reasons 1 and 4 both together
40+
* 3. data_type mismatch between table and storage.
41+
* Data is preserved through a backup table when preserve = true.
4242
*/
4343
dropped_and_recreated,
44+
45+
/**
46+
* old table is dropped and new is recreated with data loss.
47+
* Data cannot be preserved because a new NOT NULL column without
48+
* a default value is being added, making backup impossible.
49+
*/
50+
dropped_and_recreated_with_data_loss,
4451
};
4552

4653
inline std::ostream& operator<<(std::ostream& os, sync_schema_result value) {
@@ -57,6 +64,8 @@ SQLITE_ORM_EXPORT namespace sqlite_orm {
5764
return os << "old excess columns removed and new columns added";
5865
case sync_schema_result::dropped_and_recreated:
5966
return os << "old table dropped and recreated";
67+
case sync_schema_result::dropped_and_recreated_with_data_loss:
68+
return os << "old table dropped and recreated with data loss";
6069
}
6170
return os;
6271
}

‎include/sqlite_orm/sqlite_orm.h‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12390,11 +12390,18 @@ SQLITE_ORM_EXPORT namespace sqlite_orm {
1239012390
/**
1239112391
* old table is dropped and new is recreated. Reasons :
1239212392
* 1. delete excess columns in the table than storage if preseve = false
12393-
* 2. Lacking columns in the table cannot be added due to NULL and DEFAULT constraint
12394-
* 3. Reasons 1 and 2 both together
12395-
* 4. data_type mismatch between table and storage.
12393+
* 2. Reasons 1 and 4 both together
12394+
* 3. data_type mismatch between table and storage.
12395+
* Data is preserved through a backup table when preserve = true.
1239612396
*/
1239712397
dropped_and_recreated,
12398+
12399+
/**
12400+
* old table is dropped and new is recreated with data loss.
12401+
* Data cannot be preserved because a new NOT NULL column without
12402+
* a default value is being added, making backup impossible.
12403+
*/
12404+
dropped_and_recreated_with_data_loss,
1239812405
};
1239912406

1240012407
inline std::ostream& operator<<(std::ostream& os, sync_schema_result value) {
@@ -12411,6 +12418,8 @@ SQLITE_ORM_EXPORT namespace sqlite_orm {
1241112418
return os << "old excess columns removed and new columns added";
1241212419
case sync_schema_result::dropped_and_recreated:
1241312420
return os << "old table dropped and recreated";
12421+
case sync_schema_result::dropped_and_recreated_with_data_loss:
12422+
return os << "old table dropped and recreated with data loss";
1241412423
}
1241512424
return os;
1241612425
}
@@ -19784,11 +19793,11 @@ namespace sqlite_orm::internal {
1978419793
(dbColumnInfo.hidden == 0) == (storageColumnInfo.hidden == 0);
1978519794
if (!columnsAreEqual) {
1978619795
notEqual = true;
19787-
break;
19796+
} else {
19797+
dbTableInfo.erase(dbColumnInfoIt);
19798+
storageTableInfo.erase(storageTableInfo.begin() + storageColumnInfoIndex);
19799+
--storageColumnInfoIndex;
1978819800
}
19789-
dbTableInfo.erase(dbColumnInfoIt);
19790-
storageTableInfo.erase(storageTableInfo.begin() + storageColumnInfoIndex);
19791-
--storageColumnInfoIndex;
1979219801
} else {
1979319802
columnsToAdd.push_back(&storageColumnInfo);
1979419803
}
@@ -25037,6 +25046,7 @@ namespace sqlite_orm::internal {
2503725046

2503825047
auto dbTableInfo = this->pragma.table_xinfo(table.name);
2503925048
auto res = sync_schema_result::already_in_sync;
25049+
bool canPreserveData = true;
2504025050

2504125051
// first let's see if table with such name exists..
2504225052
auto gottaCreateTable = !this->table_exists(db, table.name);
@@ -25068,7 +25078,20 @@ namespace sqlite_orm::internal {
2506825078
}
2506925079
}
2507025080
if (gottaCreateTable) {
25071-
res = sync_schema_result::dropped_and_recreated;
25081+
// check if any new columns prevent data preservation
25082+
for (const table_xinfo* colInfo: columnsToAdd) {
25083+
if (!table.find_column_generated_storage_type(colInfo->name)) {
25084+
if (colInfo->notnull && colInfo->dflt_value.empty()) {
25085+
canPreserveData = false;
25086+
if (attempt_to_preserve) {
25087+
*attempt_to_preserve = false;
25088+
};
25089+
break;
25090+
}
25091+
}
25092+
}
25093+
res = canPreserveData ? sync_schema_result::dropped_and_recreated
25094+
: sync_schema_result::dropped_and_recreated_with_data_loss;
2507225095
} else {
2507325096
if (!columnsToAdd.empty()) {
2507425097
// extra storage columns than table columns
@@ -25084,6 +25107,7 @@ namespace sqlite_orm::internal {
2508425107
} else {
2508525108
if (colInfo->notnull && colInfo->dflt_value.empty()) {
2508625109
gottaCreateTable = true;
25110+
canPreserveData = false;
2508725111
// no matter if preserve is true or false, there is no way to preserve data, so we wont try!
2508825112
if (attempt_to_preserve) {
2508925113
*attempt_to_preserve = false;
@@ -25099,7 +25123,8 @@ namespace sqlite_orm::internal {
2509925123
res = sync_schema_result::new_columns_added;
2510025124
}
2510125125
} else {
25102-
res = sync_schema_result::dropped_and_recreated;
25126+
res = canPreserveData ? sync_schema_result::dropped_and_recreated
25127+
: sync_schema_result::dropped_and_recreated_with_data_loss;
2510325128
}
2510425129
} else {
2510525130
if (res != sync_schema_result::old_columns_removed) {
@@ -26035,6 +26060,9 @@ namespace sqlite_orm::internal {
2603526060
this->drop_create_with_loss(db, table);
2603626061
}
2603726062
res = schema_stat;
26063+
} else if (schema_stat == sync_schema_result::dropped_and_recreated_with_data_loss) {
26064+
this->drop_create_with_loss(db, table);
26065+
res = schema_stat;
2603826066
}
2603926067
}
2604026068
}

‎tests/sync_schema_tests.cpp‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ TEST_CASE("sync_schema") {
376376
}
377377
REQUIRE(syncSchemaSimulateRes == syncSchemaRes);
378378
decltype(syncSchemaRes) expected{
379-
{tableName, sync_schema_result::dropped_and_recreated},
379+
{tableName, sync_schema_result::dropped_and_recreated_with_data_loss},
380380
};
381381
REQUIRE(syncSchemaRes == expected);
382382
auto users = storage.get_all<User>();
@@ -469,6 +469,79 @@ TEST_CASE("sync_schema") {
469469
}
470470
}
471471

472+
// https://github.com/fnc12/sqlite_orm/issues/1462
473+
TEST_CASE("Distinguish dropped_and_recreated with and without backup") {
474+
struct MyTableRecord {
475+
int id = 0;
476+
std::string name;
477+
};
478+
479+
auto storagePath = "issue1462.sqlite";
480+
std::remove(storagePath);
481+
482+
SECTION("sequential schema changes (exact issue scenario)") {
483+
// Step 1: create initial table with a single column, insert data
484+
{
485+
auto storage = make_storage(storagePath, make_table("MyTable", make_column("id", &MyTableRecord::id)));
486+
storage.sync_schema(true);
487+
storage.insert(MyTableRecord{1, ""});
488+
storage.insert(MyTableRecord{2, ""});
489+
}
490+
// Step 2: add primary key — data should be preserved
491+
{
492+
auto storage =
493+
make_storage(storagePath, make_table("MyTable", make_column("id", &MyTableRecord::id, primary_key())));
494+
auto simulateRes = storage.sync_schema_simulate(true);
495+
auto syncRes = storage.sync_schema(true);
496+
REQUIRE(simulateRes == syncRes);
497+
REQUIRE(syncRes.at("MyTable") == sync_schema_result::dropped_and_recreated);
498+
499+
auto allRecords = storage.get_all<MyTableRecord>();
500+
REQUIRE(allRecords.size() == 2);
501+
}
502+
// Step 3: add NOT NULL column without default — data should be lost
503+
{
504+
auto storage = make_storage(storagePath,
505+
make_table("MyTable",
506+
make_column("id", &MyTableRecord::id, primary_key()),
507+
make_column("name", &MyTableRecord::name)));
508+
auto simulateRes = storage.sync_schema_simulate(true);
509+
auto syncRes = storage.sync_schema(true);
510+
REQUIRE(simulateRes == syncRes);
511+
REQUIRE(syncRes.at("MyTable") == sync_schema_result::dropped_and_recreated_with_data_loss);
512+
513+
auto allRecords = storage.get_all<MyTableRecord>();
514+
REQUIRE(allRecords.empty());
515+
}
516+
}
517+
518+
SECTION("combined pk change and new NOT NULL column in one step") {
519+
// create initial table with a single column, insert data
520+
{
521+
auto storage = make_storage(storagePath, make_table("MyTable", make_column("id", &MyTableRecord::id)));
522+
storage.sync_schema(true);
523+
storage.insert(MyTableRecord{1, ""});
524+
storage.insert(MyTableRecord{2, ""});
525+
}
526+
// add primary key AND NOT NULL column without default at once — data should be lost
527+
{
528+
auto storage = make_storage(storagePath,
529+
make_table("MyTable",
530+
make_column("id", &MyTableRecord::id, primary_key()),
531+
make_column("name", &MyTableRecord::name)));
532+
auto simulateRes = storage.sync_schema_simulate(true);
533+
auto syncRes = storage.sync_schema(true);
534+
REQUIRE(simulateRes == syncRes);
535+
REQUIRE(syncRes.at("MyTable") == sync_schema_result::dropped_and_recreated_with_data_loss);
536+
537+
auto allRecords = storage.get_all<MyTableRecord>();
538+
REQUIRE(allRecords.empty());
539+
}
540+
}
541+
542+
std::remove(storagePath);
543+
}
544+
472545
TEST_CASE("sync_schema_simulate") {
473546
struct Cols {
474547
int Col1 = 0;

0 commit comments

Comments
 (0)