Functionality request in sync_schema about sqlite_orm HOT 21 CLOSED

1. What do you mean 'unauthorized user access'? sqlite3 doesn't have users at all
2. sync_schema tries to preserve data if you pass true as first argument to it. 'Tries' means:

• • if there are excess tables exist in db they are ignored (not dropped)

    *  * every table from storage is compared with it's db analog and 
    
    *      * if table doesn't exist it is created
    
    *      * if table exists its colums are being compared with table_info from db and
    
    *          * if there are columns in db that do not exist in storage (excess) table will be dropped and recreated
    
    *          * if there are columns in storage that are not exist in db they will be added using `ALTER TABLE ... ADD COLUMN ...' command
    
    *          * if there is any column existing in both db and storage but differs by any of properties (type, pk, notnull, dflt_value) table will be dropped and recreated
    

If you want to add new values on adding new columns without default values please tell me which values should be filled in table if e.g. user changes schema by adding a table with new column named new_text TEXT NOT NULL ? Empty string? Why?

Feedback for sync_schema can be implemented like this: it should return std::map<std::string, sync_schema_result> where

enum class sync_schema_result{
    dropped_and_created,
    created,
    columns_changed,    //  data isn't altered
    synced,
};

Is it ok or you want something else?

3. Schema version must be managed by developer not by lib contributor cause different developers think differently and use different migrations.

4. table_exists function exists in storage_impl internal class. I can make it public if you want (add it to storage_t public section)

5. Let's make it like sync_schema_result sync_result(const std::string &tableName);. It will return same result that sync_schema does but will not affect db.

P.S. I still don't understand why sync_schema(true); isn't enough. Storage in sqlite_orm is designed to be static - it means there is no 'before table creation' and 'after table creation' events its login. Please describe your use case more detailed - maybe I'm wrong.

Thanks

from sqlite_orm.

@Ninja-007 added user_version in the latest commit - very useful feature. Thanks. Check it out in wiki

from sqlite_orm.

1. What do you mean 'unauthorized user access'? sqlite3 doesn't have users at all

There are tools which can edit sqlite database. 'unauthorized user access' means any modification(schema or content change) from external tools. In that case, the application will have undefined behaviour which is not accepted. So application should inform use. There is tight integration between database and app.

2. Yes, I do use true in sync_schema in preserve old table. It is not useless unless, I manually open database and compare between old and new table.
   I need to a database with my application. The database schema does change with time. Sqlite database schema and content should be in sync with application schema expectation. Current sync_schema(true) does add new columns without adding values. This does creates problems.
   Feedback from sync_schema is a good idea.
3. Yes, schema version should be managed by developed. My problem was storing schema version. Your user_version does solve the problem. Thanks.
4. Good idea
5. Good idea

PS: I hope answers to 1 and 2 explain it.

from sqlite_orm.

1. The only solution I see is encoding with third party tools
2. Assume you have:

struct User{
    int id;
    std::string name;
};

auto storage = make_storage("db.sqlite",
                              make_table("users",
                                    make_column("id", &User::id, primary_key()),
                                    make_column("name", &User::name)));

and you wanna add column 'last_name' not null without dropping the table just add default_value("") to column and sync_schema will add column with DEFAULT '':

auto storage = make_storage("db.sqlite",
                              make_table("users",
                                    make_column("id", &User::id, primary_key()),
                                    make_column("name", &User::name),
                                    make_column("last_name", &User::lastName, default_value("")));
storage.sync_schema();

There is no need to know schema version - just add NULL column or column with DEFAULT constraint.
3. Ok
4. Will be implemented soon
5. Will be implemented later

from sqlite_orm.

Thanks for latest code commit.

I was going through sync_schema documentation.
It does not cover usecase when Storage and Table columns do not match. Some Table has some extra columns and lacks some columns.

Example:
Table
Column
A B C

C++ Storage
A C D

What will happen in this case?

from sqlite_orm.

if preserve is true:
* column B will be dropped with data saving (data will be copied to a backup table and restored)
* if column D has NULL constraint or has DEFAULT constraint it will be added without data deleting otherwise table will be dropped and recreated
if preserve is false table will be dropped and recreated cause there is no DELETE COLUMN query in sqlite

from sqlite_orm.

Thanks. I tried to oversimplify the documentation. Please, let me know if it makes sense to you. It is more in line with usage.

Suggestion:
enum class sync_schema_result {
new_tbl_created, //created new table, table did not exist to delete
already_in_sync, //table schema is same as storage, nothing to be done
columns_add_remove //excess columns removed and lacking columns added
columns_add //lacking columns added
columns_remove //excess columns removed
};

1. if a table with given tablename exists
   If yes
   Compare table schema and Storage schema
   • If schema mismatch found

• preserve = true and any mismatch between table and Storage(excess columns in table, lacks some columns, data type mismatch) then make backup copy of the old table. If preserve = false then do not backup the table.

• remove excess columns in the table. If colummns removed then result is ( sync_schema_result::columns_remove)

• add new columns in the table. If columns added then if some columns are removed in above step
  the result is (sync_schema_result::columns_add_remove) else ( sync_schema_result::columns_add)

  • If no schema mismatch found Everthing is ok ( sync_schema_result::already_in_sync)

2. table with given table name does not exist. Create brand new table. (return sync_schema_result::new_tbl_created)

from sqlite_orm.

Nice but you missed rules about adding new column with NOT NULL and no default value - in this case table will be dropped and recreated cause sqlite doesn't know what values must be filled a column with. Developers must understand it cause sqlite3 itself unable to add column in this case

from sqlite_orm.

5. I added enum sync_schema_status and schema_status. It will return schema_status.

from sqlite_orm.

is the issue still actual? if it is please tell me what are you missing

from sqlite_orm.

I added schema_status. But how should it return schema status for all tables? It is still question for me.

from sqlite_orm.

You mean how to get schema_status without altering storage from public interface?

from sqlite_orm.

Yes, it should return std::map<std::string(table name), sync_schema_result>. How should it be implemented?
It should be for all tables in database.

from sqlite_orm.

I can do it or you can try to perform it. Take a look at sync_schema in storage_t (this is public interface) and sync_schema in storage_impl (internal interface). storage_t does nothing except obtaining sqlite3*pointer used by impl to access real database. Then it calls internal impl's sync_schema. Impl is defined just like std::tuple - it derives impl with tail parameter pack (just like russian doll inside a doll lol). Single impl stores information about a single table. You can write schema_status this way.

from sqlite_orm.

Thanks, I think, it will be hard for me. Sorry.

from sqlite_orm.

@Ninja-007 ok. I'll do it tomorrow

from sqlite_orm.

added in 16904a9. Also added maps equality checkin' in tests.cpp in testSchema function

from sqlite_orm.

done, Thanks

from sqlite_orm.

I checked your recent code. You added sync_schema_simulate function. simulate has different meaning e.g. proxy implementation. I feel schema_status instead of sync_schema_simulate is better name.

from sqlite_orm.

it is unable to create function overload with different return type only

from sqlite_orm.

ok

from sqlite_orm.