SQLite Forum

Apparent bug in ALTER TABLE
Login

Apparent bug in ALTER TABLE

(1) By Richard PArkins (rparkins) on 2021-01-04 12:11:05 [link] [source]

If a view exists on a table which does not exist, and PRAGMA LEGACY_ALTER_TABLE is currently 0, and an attempt is made to rename another table to be the one which the view refers to, sqlite reports an error.

To reproduce, run this sequence of SQL statements:-
CREATE TABLE main (column1, column2) ; CREATE VIEW sorted AS SELECT * FROM main ORDER BY column1 ; PRAGMA LEGACY_ALTER_TABLE = 1 ; ALTER TABLE main RENAME TO other ; PRAGMA LEGACY_ALTER_TABLE = 0 ; ALTER TABLE other RENAME TO main ;

The last statement reports Error: error in view sorted: no such table: main.main Unable to fetch row.

IMHO there is no error here: the statement is attempting to create the missing table, not to remove it.

Found using Sqlite: 3.28.0
Script run using github.com/rparkins999/sqliteman
OS: openSUSE Leap 15.1 Linux 4.12.14-lp151.28.87-default

(2.1) By Dan Kennedy (dan) on 2021-01-16 21:21:13 edited from 2.0 in reply to 1 [link] [source]

Without "PRAGMA legacy_alter_table = 1", the ALTER TABLE command will not even attempt to operate on schemas that contain views with missing references. So the error is being produced before ALTER TABLE even looks at the new table name.

(3) By Richard PArkins (rparkins) on 2021-01-16 20:19:28 in reply to 2.0 [link] [source]

Well, in that case the documentation is incomplete, since the description of ALTER TABLE contains no such statement.

I can understand why the implementation of the new version of ALTER TABLE needs to scan the schema to check if any other tables or views need to be updated, but the behaviour that Dan has described is not necessary to implement the new version. The legacy version scanned the schema too in the case where a column is renamed. A schema containing a view with a missing reference is valid, even if a little perverse, and might for example exist if another user is performing the sequence of actions described for making changes to a table which ALTER TABLE can't do. I found this issue when testing an application which does exactly that.

(4) By Richard PArkins (rparkins) on 2021-01-28 15:37:31 in reply to 2.1 [link] [source]

IMHO this is definitely a bug.

It introduced an unnecessary and undocumented interaction between processes working in entirely different parts of the database. Suppose for example that process A is doing the sequence described in section 5 of the ALTER TABLE documentation on page https://www.sqlite.org/lang_altertable.html. Between steps 6 and 7 any view that referenced table X now refers to a nonexistent table. If process B (perhaps belonging to a different user) attempts to do an ALTER TABLE in another part of the database at this point, it will get an error.

In particular if process A is actually being driven by a human running through the sequence (rather than a program), there may be quite a long time between steps 6 and 7.

Of course it is possible for process A to use an EXCLUSIVE transaction, but this is not required and would adversely affect throughput since it prevents other database connections from reading the database if not in WAL mode. It would also be possible for process A to drop all views that reference table X before doing step 6, but this is not specified, and the possibility does not change the fact that this is a perfectly valid sequence of sqlite statements which, after the change to ALTER TABLE, has an unexpected effect on another otherwise valid statement.

Why was it considered necessary to do this test? Such a test is not performed for DROP TABLE (where it might be more logical) or CREATE TABLE. If a test was needed for the existence of views referencing nonexistent tables, why not make a PRAGMA view_check by analogy with PRAGMA foreign_key_check instead of introducing it into ALTER TABLE where IMHO it does not belong? Of course ALTER TABLE has to scan the schema in order to detect whether a table or a view with the new name already exists, but it only needs to fail if a table or a view with the new name does already exist.

Incidentally dan's comment is not quite correct. The error is only generated if the ALTER TABLE statement renames a column or a table, not if it adds a column.

For example:-

-- Script started

PRAGMA legacy_alter_table = 0 ;

-- No error

--

DROP TABLE IF EXISTS table1 ;

-- No error

--

DROP TABLE IF EXISTS table2 ;

-- No error

--

CREATE TABLE table1 (t1column1 t1column2) ;

-- No error

--

CREATE TABLE table2 (t2column1 t2column2) ;

-- No error

--

CREATE VIEW view1 AS SELECT t1column1 FROM table1 ;

-- No error

--

DROP TABLE table1 ;

-- No error

--

ALTER TABLE table2 ADD COLUMN newcolumn3 ;

-- No error

--

ALTER TABLE table2 RENAME COLUMN newcolumn3 TO t2column3 ;

-- Error: error in view view1: no such table: main.table1 Unable to fetch row.

(5) By Larry Brasfield (LarryBrasfield) on 2021-01-28 18:34:07 in reply to 4 [link] [source]

IMHO this is definitely a bug.

Without getting into the whys and wherefores of your "bug" claim, do you have some complaint or improvement suggestion for how ALTER TABLE works after "PRAGMA legacy_alter_table = 1" is run or when the library is compiled with that as the default?

If your "bug" claim is a dispute with the legacy behavior, I would simply point out that there is an unknown number of library users who would be as dissatisfied as you, if not more so, were that behavior to conform to your wishes. There is an excellent chance that those users outnumber you by a lot.

(7) By Richard PArkins (rparkins) on 2021-02-08 05:39:23 in reply to 5 [link] [source]

My improvement suggestion would be that the check whether any view references a non-existent table, and the error if one does, should simply be removed from ALTER TABLE. I can't see any benefit from it at all. The existence of such a view does not, as far as I can see, prevent ALTER TABLE from working as specified. Nobody in this thread has yet offered any explanation of why the check needs to be there.

If it is desired to have such a check available somewhere (and I can see reasons why it might be wanted), it should be implemented independently from ALTER TABLE, perhaps by a PRAGMA as I suggested.

I can't see why people would be likely to object to this. The check was not in the legacy ALTER TABLE, and its existence in the post 3.26 ALTER TABLE appears to be undocumented, so normal sqlite users would not be relying on it.

I have not suggested any change to the legacy behaviour as implied by Larry. The eheck did not exist then, or at least if it was done internally it did not provoke any error.

(6) By tom (younique) on 2021-01-31 22:41:37 in reply to 4 [source]

It introduced an unnecessary and undocumented interaction between processes working in entirely different parts of the database. Suppose for example that process A is doing the sequence described in section 5 of the ALTER TABLE documentation on page https://www.sqlite.org/lang_altertable.html. Between steps 6 and 7 any view that referenced table X now refers to a nonexistent table. If process B (perhaps belonging to a different user) attempts to do an ALTER TABLE in another part of the database at this point, it will get an error.

Can this happen at all? Imho the 12-step recipe has to be executed within a transaction, so process B should not have the chance to do anything between steps 6 and 7 - only before step 1 or after step 12.

(8) By Richard PArkins (rparkins) on 2021-02-08 06:17:01 in reply to 6 [link] [source]

The question of using a transaction is already discussed in my comment. It is not possible to enclose all 12 steps in a transaction, because step #1

PRAGMA foreign_keys = off

is a no-op within a transaction as documented at https://www.sqlite.org/pragma.html#pragma_foreign_keys.