cashapp / sqldelight

SQLDelight - Generates typesafe Kotlin APIs from SQL
https://cashapp.github.io/sqldelight/
Apache License 2.0
6.04k stars 501 forks source link

Bad mapping of select query after migration #691

Closed davidbilik closed 6 years ago

davidbilik commented 6 years ago

Hi, I had this model in database version 1

CREATE TABLE agents (
    _id INTEGER PRIMARY KEY NOT NULL,
    name TEXT,
    query_text TEXT,
    section_id TEXT,
    category_id TEXT,
    category TEXT,
    price_from INTEGER as Integer,
    price_to INTEGER as Integer,
    locality TEXT,
    distance INTEGER as Integer,
    real_estate TEXT,
    domain TEXT NOT NULL,
    enabled INTEGER as Boolean NOT NULL DEFAULT 0,
    new_data INTEGER as Boolean NOT NULL DEFAULT 0,
    create_date INTEGER as Date NOT NULL,
    last_seen TEXT,
    zip_code TEXT
);

then I changed DB version to 2 and updated model with the new column section:

CREATE TABLE agents (
    _id INTEGER PRIMARY KEY NOT NULL,
    name TEXT,
    query_text TEXT,
    section_id TEXT,
    section TEXT, 
    category_id TEXT,
    category TEXT,
    price_from INTEGER as Integer,
    price_to INTEGER as Integer,
    locality TEXT,
    distance INTEGER as Integer,
    real_estate TEXT,
    domain TEXT NOT NULL,
    enabled INTEGER as Boolean NOT NULL DEFAULT 0,
    new_data INTEGER as Boolean NOT NULL DEFAULT 0,
    create_date INTEGER as Date NOT NULL,
    last_seen TEXT,
    zip_code TEXT
);

I've created alter table statement: ALTER TABLE agents ADD section TEXT;

But now my app crashes due to wrong mapping from columns to Kotlin properties, it seems shifted. This is generated mapper class (I insert image because there are parameter name hints) screen shot 2017-11-10 at 10 03 42 But if I place breakpoint inside mapper and get index of my new "section" column its 17 instead of 4 screen shot 2017-11-10 at 10 06 29 It seems that its because I specified column section in CREATE TABLE statement in fourth position but ALTER TABLE inserts it at the end. I suppose its intended behavior but I think it would be better to not depend on indexes from CREATE TABLE but instead call cursor.getColumnIndex(columnName)

AlecKazakova commented 6 years ago

once we start checking migrations we'll just fail this situation since the schema's are different. If we rely on getColumnIndex that means you have to name every column as well. You also wouldn't be able to have two columns in the result set with the same name. Neither of those are restrictions on SQLite so I'd rather not enforce those restrictions in SQLDelight

davidbilik commented 6 years ago

Ok, so what is the correct way to handle that? Place new columns to the end? Or do not use * in SELECT queries?

AlecKazakova commented 6 years ago

either place new columns at the end or recreate the table with the order you want instead of using ALTER TABLE

I think long term after we start verifying migrations we'll also have intentions in the IDE for doing this kind of "simple" migration for you (like adding a column) which should also help with this problem

davidbilik commented 6 years ago

Ok, thanks :)