SQLite.Net-PCL icon indicating copy to clipboard operation
SQLite.Net-PCL copied to clipboard

CreateTable<> does not add columns anymore

Open chingham opened this issue 10 years ago • 15 comments

There seems to be a bug in last version (2.3.0) CreateTable<> does not add missing columns anymore

chingham avatar Jun 04 '14 14:06 chingham

Could you please create a test that demonstrates the problem? Take a look at the existing tests for some examples.

oysteinkrog avatar Jun 04 '14 14:06 oysteinkrog

I don't know how to give you my Test Code I just added this to tests/CreateTableTest.cs

    private class Issue36_MyObject1
    {
        [PrimaryKey]
        public string UniqueId { get; set; }

        public byte OtherValue { get; set; }
    }
    [Table("Issue36_MyObject1")]
    private class Issue36_MyObject2 : Issue36_MyObject1
    {
        public string NewColumn { get; set; }
    }


    [Test]
    public void Issue36_MigrateTable()
    {
        using (var conn = new TestDb())
        {
            conn.CreateTable<Issue36_MyObject1>();
            conn.CreateTable<Issue36_MyObject2>();

            bool duplicate = false;
            try {
                conn.Execute(@"ALTER TABLE ""Issue36_MyObject1"" ADD COLUMN ""NewColumn"" TEXT");
            }
            catch {
                duplicate = true;
            }
            Assert.AreEqual(false, duplicate);
        }
    }

I'm not sure if this is correct for you...

chingham avatar Jun 04 '14 14:06 chingham

I just reviewed your code.

"CreateTable" checks the result of "Execute" to know whether the table has been created or not. "Execute" returns "ISQLiteApi.Changes". "ISQLiteApi.Changes" accumulates since the opening of the database.

So if you did anything before calling "CreateTable", it will believe that the table has been created. So it won't migrate.

I suggest calling Migrate in any case, since Migrate checks wich columns need to be created, no problem.

chingham avatar Jun 04 '14 15:06 chingham

Is this behavior different from that in the normal version of sqlite-net?

oysteinkrog avatar Jun 04 '14 17:06 oysteinkrog

Please, can you check if this is really a regression in this (PCL) version of sqlite.net?

oysteinkrog avatar Jul 01 '14 19:07 oysteinkrog

Any news or workaround in this bug?

GiusepeCasagrande avatar Jul 21 '14 19:07 GiusepeCasagrande

Just to confirm: "So if you did anything before calling "CreateTable", it will believe that the table has been created. So it won't migrate." is exactly what happens.

GiusepeCasagrande avatar Jul 22 '14 15:07 GiusepeCasagrande

Sorry, didn't come here for a while...

No it doesn't seem to be a regression: sqlite.net take "SQLite3.Changes" result as "affected rows" count... And it accumulates...

But it's obviously a bug Does the fact that it exists in sqlite.net, means it won't be corrected in sqlite.net PCL ? ;)

There is two way to fix this issue: Always migrate table, or return a correct "affected rows" count in Execute method

Thanks for support !

chingham avatar Jul 22 '14 16:07 chingham

Is there any update on this issue? We are also facing the same? In iPhone the Execute method returns 1 for the table that needs to be migrated, and since there is an if condition (if(count ==0 ) around the migrate table, it fails to upgrade. And I see for few tables the Execute method returns 2. What is the actual behavior here. Kindly update whether there is a fix for this issue?

Regards Mustafa

symadept avatar Sep 23 '14 11:09 symadept

I personnally forked the repo and removed the "if" on the version I'm using Ever if this bug is also present in SQLite.Net, it have obviously to be corrected

screen shot 2014-09-24 at 12 45 47

chingham avatar Sep 24 '14 10:09 chingham

@chingham I don't use this fork of SQLite-net, but in my fork I create a table exists method and use that (prior to the CREATE TABLE query) to check whether to create or migrate as opposed to checking the count:

bool TableExists(string tableName)
{
    return ExecuteScalar<int>("select count(*) from [sqlite_master] where type='table' and name='" + tableName + "'") > 0;
}

I asked a question about it on SO as it wasn't clear whether CREATE TABLE IF NOT EXISTS should actually return anything (tl;dr it doesn't): http://stackoverflow.com/questions/24785584/sqlite-create-table-if-not-exists

jamie94bc avatar Sep 26 '14 10:09 jamie94bc

Would someone submit a PR for this? It would be nice to have it fixed

molinch avatar Dec 01 '15 08:12 molinch

I use the parent repository this one is forked from and experienced the same issue. https://github.com/praeclarum/sqlite-net/issues/461

What's the common work around until (if ever) this gets fixed? It appears to be fine to simply remove the guard surrounding MigrateTable as it checks each column anyway.

aesweeting avatar Oct 18 '16 23:10 aesweeting

@adamelliottsweeting you could also migrate the entity by yourself. https://github.com/oysteinkrog/SQLite.Net-PCL/pull/218/commits/2ee41fa0f1c8688d0c9cadd95a2ba3f15a1bf12f

molinch avatar Oct 19 '16 06:10 molinch

@molinch Could you please explain a little bit how to migrate the entity? Sorry, I'm quite new on this.

vicric avatar Oct 12 '18 13:10 vicric