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

Support adding expressions for ON CONFLICT targets #692

Merged
merged 3 commits into from
Dec 14, 2023

Conversation

sumeetattree
Copy link
Contributor

@sumeetattree sumeetattree commented Aug 31, 2023

PR Info

Supports adding expressions to ON CONFLICT targets on supported databases (postgres, and sqlite);

INSERT ... ON CONFLICT ("id", LOWER("email")) DO UPDATE SET "name" = "excluded"."name"

New Features

  • Added an OnConflict::expr() and OnConflict::exprs() impls to support adding custom expressions to on_conflict targets

Breaking Changes

None

Changes

None

@sumeetattree
Copy link
Contributor Author

There is little weirdness in how OnConflict struct is initialized:

let conflict_column = OnConflict::column(Glyph::Id).to_owned(); // no need to call `new` first

let conflict_expr = OnConflict::new().expr(Func::lower(Expr::col(Glyph::Tokens))).to_owned();

.column() method acts like a constructor but .expr() requires the object to be already constructed. Changing .expr() method to also act like a constructor locks us out of using both columns and expressions in the same ON CONFLICT clause.

impl OnConflict {
    pub fn column(col) -> Self {
        // ...
    }

    pub fn expr(expr) -> Self {
        // ...
    }
}
let conflict = OnConflict::column(Glyph::Id)
    .expr(Func::lower(Expr::col(Glyph::Tokens)))  // will NOT work!
    .to_string();

On the flip side, using .expr() alone is a little cumbersome because we must first call .new().

However, doing this:

impl OnConflict {
    pub fn column(&mut self, col) -> &mut Self {
        // ...
    }

    pub fn expr(&mut self, expr) -> &mut Self {
        // ...
    }
}

is a breaking change for all .column() and .columns() consumers.

How do we reconcile these api differences?

@sumeetattree
Copy link
Contributor Author

@ikrivosheev I have tried fixing the formatting issues raised by the workflow. I think it's sorted. Can you take another look?

Copy link
Member

@tyt2y3 tyt2y3 left a comment

Choose a reason for hiding this comment

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

Sorry for the delay

@tyt2y3 tyt2y3 merged commit c21a8e1 into SeaQL:master Dec 14, 2023
Copy link

🎉 Released In 0.30.5 🎉

Thank you everyone for the contribution!
This feature is now available in the latest release. Now is a good time to upgrade!
Your participation is what makes us unique; your adoption is what drives us forward.
You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.

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.

OnConflict does not support adding a function expression to list of columns
2 participants