CreateTable<> does not add columns anymore

[chingham]

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

[oysteinkrog]

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

[chingham]

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]

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.

[oysteinkrog]

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

[oysteinkrog]

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

[GiusepeCasagrande]

Any news or workaround in this bug?

[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.

[chingham]

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 !

[symadept]

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

[chingham]

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

[jamie94bc]

@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

[molinch]

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

[aesweeting]

I use the parent repository this one is forked from and experienced the same issue.
praeclarum#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.

[molinch]

@adamelliottsweeting you could also migrate the entity by yourself.
2ee41fa