Details

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
897 ↗	(On Diff #35800)	I have some memory that this code enabled us to create a new database from scratch. Is that memory correct? If so, how is the functionality replaced?

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
897 ↗	(On Diff #35800)	According to SQLite ORM docs this function can create database. However my understanding is that we never actually used this functionality. The reason is that prior to any call to `getStorage` we first call `SQLiteQueryExecutor` constructor which internally calls `migrate` method. The `migrate` method creates database structure from scratch if necessary using SQL query. @kamil is my understanding correct?

Adding @atul since he might have context on historical SQLite setup

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
897 ↗	(On Diff #35800)	Is there any risk that the `migrate` method is missing something that is later filled in by `sqlite_orm`? It might be worth adding a step to your test plan to export a fresh database from before the deprecation and after the deprecation, and compare to see if there are any differences.

Agree that it would be good to check that the call to make_storage() isn't "filling in any gaps" from our SQL queries.

We could maybe do something like

sqlite3 blah.db .schema > blah.sql

before and after the call to make_storage() to see if there are any changes?

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
897 ↗	(On Diff #35800)	I executed plan outline by @atul in his latest comment on this diff, and output with SQL script creating schema is exactly the same before and after ORM removal.

marcin edited the test plan for this revision. (Show Details)Jan 23 2024, 7:06 AM

marcin added inline comments.

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
897 ↗	(On Diff #35800)	Updated the test plan to include those steps.

kamil added inline comments.

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
1603		are you sure the name `main` is correct? `sqlite_orm` used the same?
897 ↗	(On Diff #35800)	According to SQLite ORM docs this function can create database. However my understanding is that we never actually used this functionality. The reason is that prior to any call to `getStorage` we first call `SQLiteQueryExecutor` constructor which internally calls `migrate` method. The `migrate` method creates database structure from scratch if necessary using SQL query. @kamil is my understanding correct? yes I have some memory that this code enabled us to create a new database from scratch. Is that memory correct? If so, how is the functionality replaced? It was a time when we were using `sync_schema` but we don't do that anymore.

This revision is now accepted and ready to land.Jan 25 2024, 8:59 AM

native/cpp/CommonCpp/DatabaseManagers/SQLiteQueryExecutor.cpp
1603	are you sure the name main is correct? Yes according to `sqlite3_backup_init` documentation.. We are not working with temporary database nor with `ATTACH`-ed database. sqlite_orm used the same? Yes - there code is here.