From c5a798c15fa6ee694890c5f8e87e2c7a6001a4f4 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Sun, 8 Nov 2020 08:53:53 -0800 Subject: [PATCH] .search_sql() fully respects columns=, closes #201 Refs #192 and #197 --- sqlite_utils/db.py | 15 +++++++-------- tests/test_cli.py | 4 ++-- tests/test_fts.py | 21 +++++---------------- tests/test_tracer.py | 8 +++----- 4 files changed, 17 insertions(+), 31 deletions(-) diff --git a/sqlite_utils/db.py b/sqlite_utils/db.py index 40bcb5b..c94216e 100644 --- a/sqlite_utils/db.py +++ b/sqlite_utils/db.py @@ -1324,12 +1324,13 @@ class Table(Queryable): def search_sql(self, columns=None, order_by=None, limit=None): # Pick names for table and rank column that don't clash original = "original_" if self.name == "original" else "original" - rank = "rank" - while rank in self.columns_dict: - rank = rank + "_" columns_sql = "*" + columns_with_prefix_sql = "[{}].*".format(original) if columns: columns_sql = ", ".join("[{}]".format(c) for c in columns) + columns_with_prefix_sql = ", ".join( + "[{}].[{}]".format(original, c) for c in columns + ) fts_table = self.detect_fts() assert fts_table, "Full-text search is not configured for table '{}'".format( self.name @@ -1344,8 +1345,7 @@ class Table(Queryable): from [{dbtable}] ) select - {original}.*, - {rank_implementation} as {rank} + {columns_with_prefix} from [{original}] join [{fts_table}] on [{original}].rowid = [{fts_table}].rowid @@ -1367,10 +1367,9 @@ class Table(Queryable): dbtable=self.name, original=original, columns=columns_sql, - rank=rank, - rank_implementation=rank_implementation, + columns_with_prefix=columns_with_prefix_sql, fts_table=fts_table, - order_by=order_by or rank, + order_by=order_by or rank_implementation, limit="limit {}".format(limit) if limit else "", ).strip() diff --git a/tests/test_cli.py b/tests/test_cli.py index c82a342..0179b50 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1709,9 +1709,9 @@ def test_insert_encoding(tmpdir): [ ( None, - '[{"rowid": 2, "id": 2, "title": "Title the second", "rank": -0.5108256237659907}]\n', + '[{"rowid": 2, "id": 2, "title": "Title the second"}]\n', ), - ("--csv", "rowid,id,title,rank\n2,2,Title the second,-0.5108256237659907\n"), + ("--csv", "rowid,id,title\n2,2,Title the second\n"), ], ) def test_search(tmpdir, fts, extra_arg, expected): diff --git a/tests/test_fts.py b/tests/test_fts.py index 8cb05e9..06fac20 100644 --- a/tests/test_fts.py +++ b/tests/test_fts.py @@ -35,7 +35,6 @@ def test_enable_fts(fresh_db): "text": "tanuki are running tricksters", "country": "Japan", "not_searchable": "foo", - "rank": 0.0, } ] == list(table.search("tanuki")) assert [ @@ -44,7 +43,6 @@ def test_enable_fts(fresh_db): "text": "racoons are biting trash pandas", "country": "USA", "not_searchable": "bar", - "rank": 0.0, } ] == list(table.search("usa")) assert [] == list(table.search("bar")) @@ -71,7 +69,6 @@ def test_enable_fts_escape_table_names(fresh_db): "text": "tanuki are running tricksters", "country": "Japan", "not_searchable": "foo", - "rank": 0.0, } ] == list(table.search("tanuki")) assert [ @@ -80,7 +77,6 @@ def test_enable_fts_escape_table_names(fresh_db): "text": "racoons are biting trash pandas", "country": "USA", "not_searchable": "bar", - "rank": 0.0, } ] == list(table.search("usa")) assert [] == list(table.search("bar")) @@ -116,7 +112,6 @@ def test_populate_fts(fresh_db): "text": "racoons are biting trash pandas", "country": "USA", "not_searchable": "bar", - "rank": -0.5108256237659907, } ] == rows @@ -137,7 +132,6 @@ def test_populate_fts_escape_table_names(fresh_db): "text": "racoons are biting trash pandas", "country": "USA", "not_searchable": "bar", - "rank": -0.5108256237659907, } ] == list(table.search("usa")) @@ -198,7 +192,6 @@ def test_enable_fts_with_triggers(fresh_db): "text": "tanuki are running tricksters", "country": "Japan", "not_searchable": "foo", - "rank": 0.0, } ] table.insert(search_records[1]) @@ -210,7 +203,6 @@ def test_enable_fts_with_triggers(fresh_db): "text": "racoons are biting trash pandas", "country": "USA", "not_searchable": "bar", - "rank": 0.0, } ] assert [] == list(table.search("bar")) @@ -379,15 +371,14 @@ def test_enable_fts_replace_does_nothing_if_args_the_same(): " from [books]\n" ")\n" "select\n" - " original.*,\n" - " [books_fts].rank as rank\n" + " [original].*\n" "from\n" " [original]\n" " join [books_fts] on [original].rowid = [books_fts].rowid\n" "where\n" " [books_fts] match :query\n" "order by\n" - " rank" + " [books_fts].rank" ), ), ( @@ -401,8 +392,7 @@ def test_enable_fts_replace_does_nothing_if_args_the_same(): " from [books]\n" ")\n" "select\n" - " original.*,\n" - " [books_fts].rank as rank\n" + " [original].[title]\n" "from\n" " [original]\n" " join [books_fts] on [original].rowid = [books_fts].rowid\n" @@ -424,15 +414,14 @@ def test_enable_fts_replace_does_nothing_if_args_the_same(): " from [books]\n" ")\n" "select\n" - " original.*,\n" - " rank_bm25(matchinfo([books_fts], 'pcnalx')) as rank\n" + " [original].[title]\n" "from\n" " [original]\n" " join [books_fts] on [original].rowid = [books_fts].rowid\n" "where\n" " [books_fts] match :query\n" "order by\n" - " rank" + " rank_bm25(matchinfo([books_fts], 'pcnalx'))" ), ), ], diff --git a/tests/test_tracer.py b/tests/test_tracer.py index 318d45e..094551a 100644 --- a/tests/test_tracer.py +++ b/tests/test_tracer.py @@ -44,11 +44,9 @@ def test_with_tracer(): with db.tracer(tracer): list(db["dogs"].search("Cleopaws")) - assert len(collected) == 7 + assert len(collected) == 5 assert collected == [ ("select name from sqlite_master where type = 'view'", None), - ("select name from sqlite_master where type = 'table'", None), - ("PRAGMA table_info([dogs])", None), ( "SELECT name FROM sqlite_master\n WHERE rootpage = 0\n AND (\n sql LIKE '%VIRTUAL TABLE%USING FTS%content=%dogs%'\n OR (\n tbl_name = \"dogs\"\n AND sql LIKE '%VIRTUAL TABLE%USING FTS%'\n )\n )", None, @@ -56,11 +54,11 @@ def test_with_tracer(): ("select name from sqlite_master where type = 'view'", None), ("select sql from sqlite_master where name = ?", ("dogs_fts",)), ( - "with original as (\n select\n rowid,\n *\n from [dogs]\n)\nselect\n original.*,\n [dogs_fts].rank as rank\nfrom\n [original]\n join [dogs_fts] on [original].rowid = [dogs_fts].rowid\nwhere\n [dogs_fts] match :query\norder by\n rank", + "with original as (\n select\n rowid,\n *\n from [dogs]\n)\nselect\n [original].*\nfrom\n [original]\n join [dogs_fts] on [original].rowid = [dogs_fts].rowid\nwhere\n [dogs_fts] match :query\norder by\n [dogs_fts].rank", {"query": "Cleopaws"}, ), ] # Outside the with block collected should not be appended to db["dogs"].insert({"name": "Cleopaws"}) - assert len(collected) == 7 + assert len(collected) == 5