diff --git a/docs/changelog.rst b/docs/changelog.rst index 6e6fa30..5d6c901 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -19,6 +19,7 @@ Unreleased - Fixed a bug where a failed write statement executed with ``db.execute()`` left the driver's implicit transaction open. Every subsequent write then joined that phantom transaction, which nothing committed, so their work was silently rolled back when the connection was closed. The implicit transaction opened by a failed statement is now rolled back before the exception is raised. A failed write inside a transaction opened with ``db.begin()`` or ``db.atomic()`` leaves that transaction open and untouched, as before. - Fixed a bug where transaction-control statements prefixed with an empty statement - ``db.query("; COMMIT")`` - or a UTF-8 byte order mark slipped past the check that rejects them, committing the caller's open transaction before raising a confusing ``OperationalError``. The keyword scanner used by ``db.query()`` and ``db.execute()`` now skips leading ``;`` and byte order marks, matching what the ``sqlite3`` driver tolerates before the first token, so these statements are rejected with a ``ValueError`` without being executed. The same fix means ``db.execute("; BEGIN")`` no longer auto-commits the transaction it just opened. - Documented a limitation of ``db.query()``: a ``PRAGMA`` statement that returns no rows raises a ``ValueError`` but still takes effect, because PRAGMA statements run outside the savepoint guard used to roll back other rejected statements. Use ``db.execute()`` for row-less PRAGMA statements. +- Fixed ``pks_and_rows_where()`` raising ``AttributeError`` when called on a view, and no longer double-quotes the synthesized ``rowid`` column in its generated SQL - SQLite turns a double-quoted identifier that does not resolve into a string literal, which on a view produced a confusing ``KeyError`` instead of the ``OperationalError`` raised in 3.x. Compound primary keys returned by this method now follow ``PRIMARY KEY`` declaration order. - The ``foreign_keys=`` argument to ``create()`` and ``insert()`` accepts a mixed list of ``ForeignKey`` objects, tuples and column name strings again. In 4.0 pre-releases mixing ``ForeignKey`` objects with tuples raised a ``ValueError`` - a regression from 3.x, where ``ForeignKey`` was a ``namedtuple`` and passed the tuple checks. - ``ForeignKey`` objects are hashable again. The 4.0 change from ``namedtuple`` to dataclass accidentally made them unhashable, breaking patterns like ``set(table.foreign_keys)`` that worked in 3.x. ``ForeignKey`` is now a frozen dataclass - immutable and hashable, like the namedtuple was. - Fixed a bug where compound primary key columns were returned in table column order instead of ``PRIMARY KEY`` declaration order. For a table declared as ``CREATE TABLE other (b TEXT, a TEXT, PRIMARY KEY (a, b))`` an implicit ``FOREIGN KEY (x, y) REFERENCES other`` was introspected as referencing ``(b, a)`` when SQLite resolves it as ``(a, b)`` - running ``transform()`` on such a table then rewrote the schema with the inverted column order, silently reversing the meaning of the constraint and causing foreign key errors on valid data. ``table.pks``, compound foreign key guessing and ``transform()`` now all use the primary key declaration order, and ``transform()`` no longer reorders a compound ``PRIMARY KEY (b, a)`` into table column order. diff --git a/sqlite_utils/db.py b/sqlite_utils/db.py index 6105a64..4c7ef52 100644 --- a/sqlite_utils/db.py +++ b/sqlite_utils/db.py @@ -2023,11 +2023,22 @@ class Queryable: :param limit: Integer number of rows to limit to :param offset: Integer for SQL offset """ - column_names = [column.name for column in self.columns] - pks = self.pks - if self.use_rowid: - column_names.insert(0, "rowid") - select = ",".join(quote_identifier(column_name) for column_name in column_names) + # This method is defined on Queryable so it serves views too, which + # have no pks property - sort pk columns into declaration order here + pk_columns = sorted( + (column for column in self.columns if column.is_pk), + key=lambda column: column.is_pk, + ) + pks = [column.name for column in pk_columns] + select_parts = [quote_identifier(column.name) for column in self.columns] + if not pks: + # rowid is left unquoted: it is not a real column, and SQLite + # turns a double-quoted identifier that does not resolve into a + # string literal - on a view that would silently select the + # string 'rowid' instead of raising an error + select_parts.insert(0, "rowid") + pks = ["rowid"] + select = ",".join(select_parts) for row in self.rows_where( select=select, where=where, diff --git a/tests/test_rows.py b/tests/test_rows.py index f050d5a..46417ef 100644 --- a/tests/test_rows.py +++ b/tests/test_rows.py @@ -111,3 +111,24 @@ def test_rows_where_duplicate_select_columns_are_deduped(fresh_db): fresh_db["t"].insert({"id": 1, "name": "Cleo"}) rows = list(fresh_db["t"].rows_where(select="id, id, name")) assert rows == [{"id": 1, "id_2": 1, "name": "Cleo"}] + + +def test_pks_and_rows_where_view(fresh_db): + # pks_and_rows_where() lives on Queryable so views expose it, but + # SQLite views have no rowid - it has always failed with an + # OperationalError from the generated SQL. Guard against it failing + # earlier with an AttributeError from View lacking Table properties + from sqlite_utils.utils import sqlite3 + + fresh_db["dogs"].insert({"id": 1, "name": "Cleo"}, pk="id") + fresh_db.create_view("dog_names", "select name from dogs") + with pytest.raises(sqlite3.OperationalError): + list(fresh_db["dog_names"].pks_and_rows_where()) + + +def test_pks_and_rows_where_compound_pk_declaration_order(fresh_db): + # Compound pks are returned in PRIMARY KEY declaration order + fresh_db.execute("create table t (b text, a text, primary key (a, b))") + fresh_db["t"].insert({"a": "A", "b": "B"}) + pks_and_rows = list(fresh_db["t"].pks_and_rows_where()) + assert pks_and_rows == [(("A", "B"), {"b": "B", "a": "A"})]