Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Comments

@reivilibre
Copy link

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:

    /// 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')"#
    /// );
    /// ```
    pub fn to_tsvector<T>(expr: T, regconfig: Option<u32>) -> FunctionCall
    where
        T: 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'.

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:

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

@tuanla-mirabo
Copy link

tuanla-mirabo commented Aug 16, 2024

@reivilibre You can use Func::cust and pass 'english'::regconfig::int as an argument.

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

No branches or pull requests

2 participants