You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Exposition of Postgres functions to_tsquery, to_tsvector, phraseto_tsquery, plainto_tsquery, websearch_to_tsquery are unergonomic because they only accept the regconfig as an u32
#784
Open
reivilibre opened this issue
Jun 13, 2024
· 1 comment
The way that the Postgres functions to_tsquery, to_tsvector, phraseto_tsquery, plainto_tsquery, websearch_to_tsquery are exposed in sea-query makes them unergonomic.
Example:
/// Call `TO_TSVECTOR` function. Postgres only.////// The parameter `regconfig` represents the OID of the text search configuration./// If the value is `None` the argument is omitted from the query, and hence the database default used.////// # Examples////// ```/// use sea_query::{tests_cfg::*, *};////// let query = Query::select()/// .expr(PgFunc::to_tsvector("a b", None))/// .to_owned();////// assert_eq!(/// query.to_string(PostgresQueryBuilder),/// r#"SELECT TO_TSVECTOR('a b')"#/// );/// ```pubfnto_tsvector<T>(expr:T,regconfig:Option<u32>) -> FunctionCallwhereT:Into<SimpleExpr>,{match regconfig {Some(config) => {let config = SimpleExpr::Value(config.into());FunctionCall::new(Function::PgFunction(PgFunction::ToTsvector)).args([config, expr.into()])}None => FunctionCall::new(Function::PgFunction(PgFunction::ToTsvector)).arg(expr),}}
The problem is that it accepts the regconfig as a u32, whereas in my experience from other code bases and from the code examples in Postgres' own manual, it is more conventional to pass this as a string such as 'english' or 'simple'.
It looks like you'd have to issue queries such as the following to look it up before making the query:
postgres=# SELECT 'english'::regconfig::int; -- server 1
int4
-------14123
(1 row)
But it begs the question: why? This needs an extra round trip and makes the code harder to read.
I also note that the functions accept an Option<u32>, not a Option<SimpleExpr> or so, meaning there's no way to avoid the extra round trip — you can't build an expression equivalent to 'english'::regconfig::int and pass it in in lieu of the magic number.
Not only this, but the number is different on different database servers, which makes this a hazard for applications that may connect to multiple database servers (as a valid idea might be to perform the lookups on startup to avoid the extra round trip for every query, but then you have to remember which numbers are for which servers):
postgres=# SELECT 'english'::regconfig::int; -- server 2
int4
-------13169
(1 row)
I am not sure about whether backwards compatibility is needed or not or whether this would be accepted, but my suggestion would be to let people pass in strings here instead.
Versions
0.31.0-rc.7
The text was updated successfully, but these errors were encountered:
Description
The way that the Postgres functions
to_tsquery
,to_tsvector
,phraseto_tsquery
,plainto_tsquery
,websearch_to_tsquery
are exposed in sea-query makes them unergonomic.Example:
The problem is that it accepts the regconfig as a
u32
, whereas in my experience from other code bases and from the code examples in Postgres' own manual, it is more conventional to pass this as a string such as'english'
or'simple'
.Ref: https://www.postgresql.org/docs/current/textsearch-controls.html#TEXTSEARCH-PARSING-QUERIES
It looks like you'd have to issue queries such as the following to look it up before making the query:
But it begs the question: why? This needs an extra round trip and makes the code harder to read.
I also note that the functions accept an
Option<u32>
, not aOption<SimpleExpr>
or so, meaning there's no way to avoid the extra round trip — you can't build an expression equivalent to'english'::regconfig::int
and pass it in in lieu of the magic number.Not only this, but the number is different on different database servers, which makes this a hazard for applications that may connect to multiple database servers (as a valid idea might be to perform the lookups on startup to avoid the extra round trip for every query, but then you have to remember which numbers are for which servers):
I am not sure about whether backwards compatibility is needed or not or whether this would be accepted, but my suggestion would be to let people pass in strings here instead.
Versions
0.31.0-rc.7
The text was updated successfully, but these errors were encountered: