Fix check for name of first column

[jooert]

I could not edit a simple SQLite database using sqlite-mode-extras-edit-row-field because it would only print "First column must be id" (similar to #5).

As far as I understand the code, this check should make sure that the value from the first column is actually an id before updating the row. sqlite-mode-extras-edit-row-field then proceeds to use this first-column value to select the row to update via the rowid (... WHERE rowid = ?).

So shouldn't the check then be about rowid instead of id? Additionally, according to https://github.com/emacs-mirror/emacs/blob/master/lisp/sqlite-mode.el#L174, rowid (and not id, which might not even exist) will actually always be the first column in the list of data.

When I modify the code as such, field-editing is working for me, so I guess this fixes #5.

[xenodium]

Thanks for the PR! It's been a while since I looked at this code.

IIRC, I placed those as safety measures since the code was fairly experimental and it worked for the use-case I had at the time. To make it more flexible, we possibly need to check if the first column is a key rather than check for id name. Relying on column name doesn't seem very reliable. For example, running on the following DB with the patch has a similar issue:

First column must be ’rowid’

CREATE TABLE futurama_characters (
    id INTEGER PRIMARY KEY,
    name TEXT,
    job TEXT,
    origin TEXT
);

My SQL knowledge is fairly limited, so I'm happy to take recommendations.

[jooert]

Yes, you are right. I was under the impression that every SQLite table has an implicit INTEGER PRIMARY KEY called rowid (sqlite-mode itself seems to assume the same and always queries for rowid when displaying data, see https://github.com/emacs-mirror/emacs/blob/master/lisp/sqlite-mode.el#L174).
Apparently, that is not true in general, but often (see https://sqlite.org/rowidtable.html). When there is an explicit INTEGER PRIMARY KEY like in your example, that explicit key becomes an alias for rowid. Apparently, sqlite then returns the column name of that key instead of rowid even when querying for rowid (but that seems to be unspecified, see https://sqlite.org/c3ref/column_name.html). Therefore, there is no rowid column in the table header of your example (but id is shown twice).

I see three relevant cases:

1. WITHOUT ROWID tables: Those have been explicitly created to not have a rowid but instead need to have another primary key.
2. "rowid tables" without explicit INTEGER PRIMARY KEY: That was my initial use case.
3. "rowid tables" with explicit INTEGER PRIMARY KEY: That is your example.

I have created a test database using the following schema creating three corresponding tables:

CREATE TABLE without_rowid (
    name TEXT PRIMARY KEY,
    job TEXT
) WITHOUT ROWID;

CREATE TABLE no_pk (
    name TEXT,
    job TEXT
);

CREATE TABLE integer_pk (
    id INTEGER PRIMARY KEY,
    name TEXT,
    job TEXT
);

sqlite-mode cannot show table data for without_rowid because of the missing rowid column and fails with sqlite-mode-list-data: Database error: ("SQL logic error" "no such column: rowid" 1 1). So case 1 does not work in sqlite-mode at the moment.

For no_pk, the first column display in sqlite-mode is rowid, for integer_pk the first (two) column(s) are id.
To find the position of the primary key column(s), PRAGMA table_info(<TABLE_NAME>); can be used, see https://sqlite.org/pragma.html#pragma_table_info. For no_pk, this does not return the automatic rowid column, though.

When I find more time, I will try to update the code to use this information to select the correct primary key column(s).

[xenodium]

Thanks for looking into it. Sounds like we may need changes upstream too (in sqlite-mode.el).