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

OnConflict does not support adding a function expression to list of columns #678

Closed
sumeetattree opened this issue Aug 18, 2023 · 3 comments · Fixed by #692
Closed

OnConflict does not support adding a function expression to list of columns #678

sumeetattree opened this issue Aug 18, 2023 · 3 comments · Fixed by #692

Comments

@sumeetattree
Copy link
Contributor

Description

I was trying to add a function expression to the column list in an OnConflict statement. But this does not seem to be possible. Only objects that implement IntoIden are acceptable in the OnConflict::columns or the OnConflict::column method.

Postgres supports adding function expressions in the column list on the ON CONFLICT clause on insert statements. This functionality seems to be missing from sea query.

How do I achieve this? Is there way to do this that I might have overlooked?

INSERT INTO posts (name, identifier, content)
VALUES ('Getting Started', 'getting-started', 'Some sample content')
ON CONFLICT (lower(identifier)) -- We want to add a function expression to the column list here
DO NOTHING;

Expected Behavior

let mut statement = Query::insert()
    .into_table(PostSchema::Table)
    .columns([PostSchema::Name, PostSchema::Identifier, PostSchema::Content])
    .values(["Getting Started", "getting-started", "Some sample content"])
    .on_conflict(
        OnConflict::column(
            Expr::expr(Func::lower(PostSchema::Identifier)) // This does not seem to be possible as SimpleExpr does not implement `IntoIden`
        )
        .do_nothing()
        .to_owned(),
    );

Adding an Alias to the column does not seem to be working correctly.

OnConflict::column(Alias::new("lower(identifier)"));

Postgres returns "lower(identifier)" is not a valid column name. It is due to the " around the column name when it's converted to an sql statement.

Actual Behavior

Compile time error:

error[E0277]: the trait bound `sea_query::Expr: Iden` is not satisfied
   --> src/main.rs
   |
   |       OnConflict::column(
   |       ------------------ required by a bound introduced by this call
...
   |       Expr::expr(Func::lower(Expr::col(PostSchema::Identifier))),
   |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Iden` is not implemented for `sea_query::Expr`

Versions

cargo tree | grep sea-
    │   ├── sea-query v0.29.0 (https://github.com/SeaQL/sea-query.git?branch=master#1c9a6656)
    │   │   ├── sea-query-derive v0.3.0 (proc-macro) (https://github.com/SeaQL/sea-query.git?branch=master#1c9a6656)
    ├── sea-query v0.29.0 (https://github.com/SeaQL/sea-query.git?branch=master#1c9a6656) (*)
    ├── sea-query-binder v0.4.0 (https://github.com/SeaQL/sea-query.git?branch=master#1c9a6656)
    │   ├── sea-query v0.29.0 (https://github.com/SeaQL/sea-query.git?branch=master#1c9a6656) (*)

OS: WSL2 running Ubuntu 20.04.6 LTS

Suggestions

We could add a .expr() method on OnConflict just like Query::select().expr() that let's us pass custom expressions.

let conflict = OnConflict::columns([ /* ... */ ]).expr(Func::lower(Expr::col(PostSchema::Identifier)));

I could take a stab at it if you are interested in adding this functionality. I would be interested in knowing if this was overlooked or was never planned to be added due to a specific reason.

Thanks again for this awesome library! I have been really enjoying working with it.

@ikrivosheev
Copy link
Member

@sumeetattree hello! Thank you for the issue. Yes, it looks reasonable for me. Could you want to create PR?

@sumeetattree
Copy link
Contributor Author

Hi @ikrivosheev Yes! Let me look into it. Where would I post should I need some help with this?

@ikrivosheev
Copy link
Member

You can write your questions here or we can talk in discord)

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 a pull request may close this issue.

2 participants