Postgres only. SQLite's FTS is a separate, unrelated extension, and
CockroachDB's tsvector support is incomplete — so passing a column belonging
to either engine into any of these raises
ValueError("Only Postgres supports full text search."), following the same
pattern as the array functions. A SQLite user gets that error at query-build
time, not a confusing database error.
There's currently no way to express Postgres full text search through Piccolo,
so it has to be written as raw SQL. This adds the pieces as composable
QueryStrings.
Postgres only resolves to_tsvector(config, text) when the config is a
constant, and an expression index over it is only IMMUTABLE because of that.
So config is a str which is written into the query rather than bound.
Since it isn't parameterised, it's validated against ^[A-Za-z_][A-Za-z0-9_]*$
before it goes in, so it can't be used for injection. config=None (the
default) emits the single-argument form, which uses the database's
default_text_search_config — the docstring notes that this form is STABLE
rather than IMMUTABLE, so it can't be used in an expression index.
Sanitising user input
to_tsquery raises a database error on a stray operator, so it's the wrong
thing to hand a raw search box to. PlainToTsQuery and WebsearchToTsQuery
accept any text, so they're the safe entry points — that's called out in
ToTsQuery's docstring with a .. warning:: pointing at them.
A question for you: the one thing to_tsquery can do that the safe
variants can't is prefix matching (python:*), which is what you want for
type-ahead search. Doing that safely means a helper which strips the input to
word characters, ANDs them, and appends :* to the last word. I've left it
out — it's a policy decision rather than a wrapper around a Postgres function,
and it's the kind of thing that belongs in the docs as a recipe if you don't
want it in the library. Happy to add it if you do.
Deliberately left out
ts_rank / ts_rank_cd / ts_headline — the obvious follow-ups, but they
have fussier signatures (normalization flags, option strings) and this PR is
already the whole matching path.
tsvector / tsquery column types, and GIN index support for them.
Right now the vector has to be computed per query, or the user has to add the
column and index through a raw migration. That's the bigger piece of work and
I didn't want to bundle it in.
Relaxing the engine check for CockroachDB — it has had partial tsvector
support since v23.2, but I have no instance to verify against, so I've been
conservative. It's a one-line change to check_engine if you'd rather allow
it.
Naming
Matches is the least obvious name here — the operator has no function name in
Postgres. Alternatives would be TsMatch or TextSearchMatch. Happy to rename.
Testing
New file tests/query/functions/test_text_search.py — 7 Postgres-only
behaviour tests plus config validation and a check that the wrong engine
raises.
Full suite against Postgres — 1 failed, 891 passed, 34 skipped. The one
failure is tests/table/test_update.py::TestOperators::test_operators, which
fails on master for me too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Postgres only. SQLite's FTS is a separate, unrelated extension, and
CockroachDB's tsvector support is incomplete — so passing a column belonging
to either engine into any of these raises
ValueError("Only Postgres supports full text search."), following the same
pattern as the array functions. A SQLite user gets that error at query-build
time, not a confusing database error.
There's currently no way to express Postgres full text search through Piccolo,
so it has to be written as raw SQL. This adds the pieces as composable
QueryStrings.
The text search config can't be a query parameter
Postgres only resolves to_tsvector(config, text) when the config is a
constant, and an expression index over it is only IMMUTABLE because of that.
So config is a str which is written into the query rather than bound.
Since it isn't parameterised, it's validated against ^[A-Za-z_][A-Za-z0-9_]*$
before it goes in, so it can't be used for injection. config=None (the
default) emits the single-argument form, which uses the database's
default_text_search_config — the docstring notes that this form is STABLE
rather than IMMUTABLE, so it can't be used in an expression index.
Sanitising user input
to_tsquery raises a database error on a stray operator, so it's the wrong
thing to hand a raw search box to. PlainToTsQuery and WebsearchToTsQuery
accept any text, so they're the safe entry points — that's called out in
ToTsQuery's docstring with a .. warning:: pointing at them.
A question for you: the one thing to_tsquery can do that the safe
variants can't is prefix matching (python:*), which is what you want for
type-ahead search. Doing that safely means a helper which strips the input to
word characters, ANDs them, and appends :* to the last word. I've left it
out — it's a policy decision rather than a wrapper around a Postgres function,
and it's the kind of thing that belongs in the docs as a recipe if you don't
want it in the library. Happy to add it if you do.
Deliberately left out
have fussier signatures (normalization flags, option strings) and this PR is
already the whole matching path.
Right now the vector has to be computed per query, or the user has to add the
column and index through a raw migration. That's the bigger piece of work and
I didn't want to bundle it in.
support since v23.2, but I have no instance to verify against, so I've been
conservative. It's a one-line change to check_engine if you'd rather allow
it.
Naming
Matches is the least obvious name here — the operator has no function name in
Postgres. Alternatives would be TsMatch or TextSearchMatch. Happy to rename.
Testing
New file tests/query/functions/test_text_search.py — 7 Postgres-only
behaviour tests plus config validation and a check that the wrong engine
raises.
Full suite against Postgres — 1 failed, 891 passed, 34 skipped. The one
failure is tests/table/test_update.py::TestOperators::test_operators, which
fails on master for me too.