WITHOUT ROWID for TEXT column

[fu-hsi]

SqliteBrowser allows using WITHOUT ROWID only for INTEGER column.
SQLite does not have this limitation:
https://www.sqlite.org/withoutrowid.html

[justinclift]

@MKleusberg ?

[fu-hsi]

[rp-]

should be fixed:
cecf681

[justinclift]

Cool. 😄

@fu-hsi Do you have time to test the next nightly build, and let us know if it works ok now?

[fu-hsi]

Of course.

[fu-hsi]

My current table schema:

CREATE TABLE `test` (
    `Field1`    TEXT NOT NULL,
    `Field2`    INTEGER NOT NULL DEFAULT 1,
    PRIMARY KEY(Field1)
) WITHOUT ROWID;

When I try modify the table schema and check Unique checkbox, application not respond and crashes.

Inserting data from Browse Data tab don't work, I don't know why...
When I click New record I received:

Manually insert works:

Query executed successfully: insert into test values('bla',1); (took 0ms)
Query executed successfully: insert into test values('bla2',1); (took 0ms)

Look on SQL Log on right panel...
SELECT MAX() from TEXT field??

I am usung now SQLiteStudio since two weeks (sorry guys :)) and works like a charm :)

[justinclift]

@rp- Thoughts? 😄

@fu-hsi:

I am usung now SQLiteStudio since two weeks (sorry guys :)) and works like a charm :)

That's no worries. When there's a problem that needs solving I often use whatever works first too. 😉

[fu-hsi]

Editing and deleting also doesn't work :)
Even if I change Primary key to NUMERIC.

Log:

UPDATE `test` SET `Field1`=? WHERE `Field1`=0;
DELETE FROM `test` WHERE `Field1`=0;

Field Field1 always equal 0.

When I create new table with NUMERIC key, then it works.

[justinclift]

@rp- Sounds like the fix really didn't work. 😉

[fu-hsi]

This is more complex than the programmer thought.
Many functions operates on numeric keys, and now... all must be changed? :)
Maybe I'm wrong and the key can not be text...?

[rp-]

well, all i did was enabling to have tables without rowid and pks of any type, as allowed by sqlite3.
I think we still don't really support editing tables without rowid, because most of our code depends on it.

[MKleusberg]

At least the thing with the crash when adding a UNIQUE constraint in that special case seems to be a bug in SQLite itself. The same happens when executing this code using the command line utility:

CREATE TABLE `test` (
    `Field1`    TEXT UNIQUE NOT NULL,
    `Field2`    INTEGER NOT NULL DEFAULT 1,
    PRIMARY KEY(Field1)
) WITHOUT ROWID;

In the latest development version of SQLite, however, I couldn't reproduce this anymore.

[justinclift]

Ok. What's the right way forward here? Should we go back to having "without rowid" only for numeric columns?

[MKleusberg]

Ok, to quickly summarise the situation:

• The UNIQUE TEXT primary key thing is a SQLite bug which seems to be fixed in the latest version. So I suggest ignoring this one.
• Without Rowid table are more or less supported, so editing, deleting, adding, etc. of rows works most of the times. I don't think there's any code left which really depends on the rowid column, the only remaining issues may occur because we don't handle some situations not entirely correct yet.
• One of this was the text primary key thing. The SQLite documentation says the primary key column in a without rowid table must be numeric but apparently this isn't the case. @fu-hsi is right that most of our code assumed the primary key was numeric which caused a lot of trouble when it wasn't. The commit I've just pushed should fix these issues, so we handle non-numeric primary keys correctly.

So I think we don't need to go back because it as easy enough to fix the problematic parts. The new code needs a bit of testing (in a scenario like the one described in this issue as well as under normal circumstances) but should fix all the problems described in this issue (except for the SQLite bug that is of course).

[justinclift]

Cool. 😄

Is this something we should ask @fu-hsi to test again in the next nightly build?

[MKleusberg]

Yes definitely, if he has the time and motivation 😄
@fu-hsi Do you? 😉

[MKleusberg]

Somebody willing to test this? If not I'd suggest closing this issue and if there should be any more problems with this it can always be reopened.

[justinclift]

Closing it for now. @fu-hsi Do you have a few minutes to test our latest nightly build, and let us know if it's not working after all?