Skip to content

Escape the table name in populate_fts and search.#56

Closed
amjith wants to merge 1 commit intosimonw:masterfrom
amjith:master
Closed

Escape the table name in populate_fts and search.#56
amjith wants to merge 1 commit intosimonw:masterfrom
amjith:master

Conversation

@amjith
Copy link
Contributor

@amjith amjith commented Sep 1, 2019

The table names weren't escaped using double quotes in the populate_fts method.

Reproducible case:

>>> import sqlite_utils
>>> db = sqlite_utils.Database("abc.db")
>>> db["http://example.com"].insert_all([
...     {"id": 1, "age": 4, "name": "Cleo"},
...     {"id": 2, "age": 2, "name": "Pancakes"}
... ], pk="id")
<Table http://example.com (id, age, name)>
>>> db["http://example.com"].enable_fts(["name"])
Traceback (most recent call last):
  File "<input>", line 1, in <module>
    db["http://example.com"].enable_fts(["name"])
  File "/home/amjith/.virtualenvs/itsysearch/lib/python3.7/site-packages/sqlite_utils/db.py", l
ine 705, in enable_fts
    self.populate_fts(columns)
  File "/home/amjith/.virtualenvs/itsysearch/lib/python3.7/site-packages/sqlite_utils/db.py", l
ine 715, in populate_fts
    self.db.conn.executescript(sql)
sqlite3.OperationalError: unrecognized token: ":"
>>> 

INSERT INTO "{table}_fts" (rowid, {columns})
SELECT rowid, {columns} FROM {table};
SELECT rowid, {columns} FROM "{table}";
""".format(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been using [...] escaping instead of "..." escaping elsewhere, so let's do that here as well for consistency.

@simonw
Copy link
Owner

simonw commented Sep 2, 2019

Good spot, thanks!

Would be useful to add a test as well, derived from your above example.

@amjith
Copy link
Contributor Author

amjith commented Sep 2, 2019

I have updated the other PR with the changes from this one and added tests. I have also changed the escaping from double quotes to brackets.

@amjith amjith closed this Sep 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants