Repository: SeaQL/sea-query (language: rust)

Issue:
OnConflict does not support adding a function expression to list of columns
## 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?

```sql
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

```rs
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.

```rs
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:
```rs
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.
```rs
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.


Discussion / hints:
@sumeetattree hello! Thank you for the issue. Yes, it looks reasonable for me. Could you want to create PR? 
Hi @ikrivosheev Yes! Let me look into it. Where would I post should I need some help with this?
You can write your questions here or we can talk in discord)

Fix patch:
diff --git a/src/backend/mysql/query.rs b/src/backend/mysql/query.rs
index d9ac461ec..f652e1400 100644
--- a/src/backend/mysql/query.rs
+++ b/src/backend/mysql/query.rs
@@ -59,7 +59,7 @@ impl QueryBuilder for MysqlQueryBuilder {
         sql.push_param(value.clone(), self as _);
     }
 
-    fn prepare_on_conflict_target(&self, _: &Option<OnConflictTarget>, _: &mut dyn SqlWriter) {
+    fn prepare_on_conflict_target(&self, _: &[OnConflictTarget], _: &mut dyn SqlWriter) {
         // MySQL doesn't support declaring ON CONFLICT target.
     }
 
diff --git a/src/backend/query_builder.rs b/src/backend/query_builder.rs
index fd8c7677d..dab9aa535 100644
--- a/src/backend/query_builder.rs
+++ b/src/backend/query_builder.rs
@@ -1108,7 +1108,7 @@ pub trait QueryBuilder:
     fn prepare_on_conflict(&self, on_conflict: &Option<OnConflict>, sql: &mut dyn SqlWriter) {
         if let Some(on_conflict) = on_conflict {
             self.prepare_on_conflict_keywords(sql);
-            self.prepare_on_conflict_target(&on_conflict.target, sql);
+            self.prepare_on_conflict_target(&on_conflict.targets, sql);
             self.prepare_on_conflict_condition(&on_conflict.target_where, sql);
             self.prepare_on_conflict_action(&on_conflict.action, sql);
             self.prepare_on_conflict_condition(&on_conflict.action_where, sql);
@@ -1119,24 +1119,30 @@ pub trait QueryBuilder:
     /// Write ON CONFLICT target
     fn prepare_on_conflict_target(
         &self,
-        on_conflict_target: &Option<OnConflictTarget>,
+        on_conflict_targets: &[OnConflictTarget],
         sql: &mut dyn SqlWriter,
     ) {
-        if let Some(target) = on_conflict_target {
+        if on_conflict_targets.is_empty() {
+            return;
+        }
+
+        write!(sql, "(").unwrap();
+        on_conflict_targets.iter().fold(true, |first, target| {
+            if !first {
+                write!(sql, ", ").unwrap()
+            }
             match target {
-                OnConflictTarget::ConflictColumns(columns) => {
-                    write!(sql, "(").unwrap();
-                    columns.iter().fold(true, |first, col| {
-                        if !first {
-                            write!(sql, ", ").unwrap()
-                        }
-                        col.prepare(sql.as_writer(), self.quote());
-                        false
-                    });
-                    write!(sql, ")").unwrap();
+                OnConflictTarget::ConflictColumn(col) => {
+                    col.prepare(sql.as_writer(), self.quote());
+                }
+
+                OnConflictTarget::ConflictExpr(expr) => {
+                    self.prepare_simple_expr(expr, sql);
                 }
             }
-        }
+            false
+        });
+        write!(sql, ")").unwrap();
     }
 
     #[doc(hidden)]
diff --git a/src/query/on_conflict.rs b/src/query/on_conflict.rs
index 280a8491f..47d45c0f6 100644
--- a/src/query/on_conflict.rs
+++ b/src/query/on_conflict.rs
@@ -2,7 +2,7 @@ use crate::{ConditionHolder, DynIden, IntoCondition, IntoIden, SimpleExpr};
 
 #[derive(Debug, Clone, Default, PartialEq)]
 pub struct OnConflict {
-    pub(crate) target: Option<OnConflictTarget>,
+    pub(crate) targets: Vec<OnConflictTarget>,
     pub(crate) target_where: ConditionHolder,
     pub(crate) action: Option<OnConflictAction>,
     pub(crate) action_where: ConditionHolder,
@@ -11,8 +11,10 @@ pub struct OnConflict {
 /// Represents ON CONFLICT (upsert) targets
 #[derive(Debug, Clone, PartialEq)]
 pub enum OnConflictTarget {
-    /// A list of columns with unique constraint
-    ConflictColumns(Vec<DynIden>),
+    /// A column
+    ConflictColumn(DynIden),
+    /// An expression `(LOWER(column), ...)`
+    ConflictExpr(SimpleExpr),
 }
 
 /// Represents ON CONFLICT (upsert) actions
@@ -55,15 +57,86 @@ impl OnConflict {
         I: IntoIterator<Item = C>,
     {
         Self {
-            target: Some(OnConflictTarget::ConflictColumns(
-                columns.into_iter().map(IntoIden::into_iden).collect(),
-            )),
+            targets: columns
+                .into_iter()
+                .map(|c| OnConflictTarget::ConflictColumn(c.into_iden()))
+                .collect(),
             target_where: ConditionHolder::new(),
             action: None,
             action_where: ConditionHolder::new(),
         }
     }
 
+    /// Set ON CONFLICT target expression
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use sea_query::{tests_cfg::*, *};
+    ///
+    /// let query = Query::insert()
+    ///     .into_table(Glyph::Table)
+    ///     .columns([Glyph::Aspect, Glyph::Image])
+    ///     .values_panic(["abcd".into(), 3.1415.into()])
+    ///     .on_conflict(
+    ///         OnConflict::new()
+    ///             .expr(Expr::col(Glyph::Id))
+    ///             .update_column(Glyph::Aspect)
+    ///             .value(Glyph::Image, Expr::val(1).add(2))
+    ///             .to_owned(),
+    ///     )
+    ///     .to_owned();
+    ///
+    /// assert_eq!(
+    ///     query.to_string(MysqlQueryBuilder),
+    ///     [
+    ///         r#"INSERT INTO `glyph` (`aspect`, `image`)"#,
+    ///         r#"VALUES ('abcd', 3.1415)"#,
+    ///         r#"ON DUPLICATE KEY UPDATE `aspect` = VALUES(`aspect`), `image` = 1 + 2"#,
+    ///     ]
+    ///     .join(" ")
+    /// );
+    /// assert_eq!(
+    ///     query.to_string(PostgresQueryBuilder),
+    ///     [
+    ///         r#"INSERT INTO "glyph" ("aspect", "image")"#,
+    ///         r#"VALUES ('abcd', 3.1415)"#,
+    ///         r#"ON CONFLICT ("id") DO UPDATE SET "aspect" = "excluded"."aspect", "image" = 1 + 2"#,
+    ///     ]
+    ///     .join(" ")
+    /// );
+    /// assert_eq!(
+    ///     query.to_string(SqliteQueryBuilder),
+    ///     [
+    ///         r#"INSERT INTO "glyph" ("aspect", "image")"#,
+    ///         r#"VALUES ('abcd', 3.1415)"#,
+    ///         r#"ON CONFLICT ("id") DO UPDATE SET "aspect" = "excluded"."aspect", "image" = 1 + 2"#,
+    ///     ]
+    ///     .join(" ")
+    /// );
+    /// ```
+    pub fn expr<T>(&mut self, expr: T) -> &mut Self
+    where
+        T: Into<SimpleExpr>,
+    {
+        Self::exprs(self, [expr])
+    }
+
+    /// Set multiple target expressions for ON CONFLICT. See [`OnConflict::expr`]
+    pub fn exprs<I, T>(&mut self, exprs: I) -> &mut Self
+    where
+        T: Into<SimpleExpr>,
+        I: IntoIterator<Item = T>,
+    {
+        self.targets.append(
+            &mut exprs
+                .into_iter()
+                .map(|e: T| OnConflictTarget::ConflictExpr(e.into()))
+                .collect(),
+        );
+        self
+    }
+
     pub fn do_nothing(&mut self) -> &mut Self {
         self.action = Some(OnConflictAction::DoNothing);
         self


Test patch:
diff --git a/tests/postgres/query.rs b/tests/postgres/query.rs
index 1a0d87425..5df9ed281 100644
--- a/tests/postgres/query.rs
+++ b/tests/postgres/query.rs
@@ -1436,6 +1436,87 @@ fn insert_on_conflict_6() {
     );
 }
 
+#[test]
+#[allow(clippy::approx_constant)]
+fn insert_on_conflict_7() {
+    assert_eq!(
+        Query::insert()
+            .into_table(Glyph::Table)
+            .columns([Glyph::Aspect, Glyph::Image])
+            .values_panic([
+                "04108048005887010020060000204E0180400400".into(),
+                3.1415.into(),
+            ])
+            .on_conflict(
+                OnConflict::new()
+                    .expr(Expr::col(Glyph::Id))
+                    .update_column(Glyph::Aspect)
+                    .to_owned()
+            )
+            .to_string(PostgresQueryBuilder),
+        [
+            r#"INSERT INTO "glyph" ("aspect", "image")"#,
+            r#"VALUES ('04108048005887010020060000204E0180400400', 3.1415)"#,
+            r#"ON CONFLICT ("id") DO UPDATE SET "aspect" = "excluded"."aspect""#,
+        ]
+        .join(" ")
+    );
+}
+
+#[test]
+#[allow(clippy::approx_constant)]
+fn insert_on_conflict_8() {
+    assert_eq!(
+        Query::insert()
+            .into_table(Glyph::Table)
+            .columns([Glyph::Aspect, Glyph::Image])
+            .values_panic([
+                "04108048005887010020060000204E0180400400".into(),
+                3.1415.into(),
+            ])
+            .on_conflict(
+                OnConflict::new()
+                    .exprs([Expr::col(Glyph::Id), Expr::col(Glyph::Aspect)])
+                    .update_column(Glyph::Aspect)
+                    .to_owned()
+            )
+            .to_string(PostgresQueryBuilder),
+        [
+            r#"INSERT INTO "glyph" ("aspect", "image")"#,
+            r#"VALUES ('04108048005887010020060000204E0180400400', 3.1415)"#,
+            r#"ON CONFLICT ("id", "aspect") DO UPDATE SET "aspect" = "excluded"."aspect""#,
+        ]
+        .join(" ")
+    );
+}
+
+#[test]
+#[allow(clippy::approx_constant)]
+fn insert_on_conflict_9() {
+    assert_eq!(
+        Query::insert()
+            .into_table(Glyph::Table)
+            .columns([Glyph::Aspect, Glyph::Image])
+            .values_panic([
+                "04108048005887010020060000204E0180400400".into(),
+                3.1415.into(),
+            ])
+            .on_conflict(
+                OnConflict::column(Glyph::Id)
+                    .expr(Func::lower(Expr::col(Glyph::Tokens)))
+                    .update_column(Glyph::Aspect)
+                    .to_owned()
+            )
+            .to_string(PostgresQueryBuilder),
+        [
+            r#"INSERT INTO "glyph" ("aspect", "image")"#,
+            r#"VALUES ('04108048005887010020060000204E0180400400', 3.1415)"#,
+            r#"ON CONFLICT ("id", LOWER("tokens")) DO UPDATE SET "aspect" = "excluded"."aspect""#,
+        ]
+        .join(" ")
+    );
+}
+
 #[test]
 #[allow(clippy::approx_constant)]
 fn insert_returning_all_columns() {
diff --git a/tests/sqlite/query.rs b/tests/sqlite/query.rs
index d898010fe..adbe55b32 100644
--- a/tests/sqlite/query.rs
+++ b/tests/sqlite/query.rs
@@ -1367,6 +1367,87 @@ fn insert_on_conflict_6() {
     );
 }
 
+#[test]
+#[allow(clippy::approx_constant)]
+fn insert_on_conflict_7() {
+    assert_eq!(
+        Query::insert()
+            .into_table(Glyph::Table)
+            .columns([Glyph::Aspect, Glyph::Image])
+            .values_panic([
+                "04108048005887010020060000204E0180400400".into(),
+                3.1415.into(),
+            ])
+            .on_conflict(
+                OnConflict::new()
+                    .expr(Expr::col(Glyph::Id))
+                    .update_column(Glyph::Aspect)
+                    .to_owned()
+            )
+            .to_string(PostgresQueryBuilder),
+        [
+            r#"INSERT INTO "glyph" ("aspect", "image")"#,
+            r#"VALUES ('04108048005887010020060000204E0180400400', 3.1415)"#,
+            r#"ON CONFLICT ("id") DO UPDATE SET "aspect" = "excluded"."aspect""#,
+        ]
+        .join(" ")
+    );
+}
+
+#[test]
+#[allow(clippy::approx_constant)]
+fn insert_on_conflict_8() {
+    assert_eq!(
+        Query::insert()
+            .into_table(Glyph::Table)
+            .columns([Glyph::Aspect, Glyph::Image])
+            .values_panic([
+                "04108048005887010020060000204E0180400400".into(),
+                3.1415.into(),
+            ])
+            .on_conflict(
+                OnConflict::new()
+                    .exprs([Expr::col(Glyph::Id), Expr::col(Glyph::Aspect)])
+                    .update_column(Glyph::Aspect)
+                    .to_owned()
+            )
+            .to_string(PostgresQueryBuilder),
+        [
+            r#"INSERT INTO "glyph" ("aspect", "image")"#,
+            r#"VALUES ('04108048005887010020060000204E0180400400', 3.1415)"#,
+            r#"ON CONFLICT ("id", "aspect") DO UPDATE SET "aspect" = "excluded"."aspect""#,
+        ]
+        .join(" ")
+    );
+}
+
+#[test]
+#[allow(clippy::approx_constant)]
+fn insert_on_conflict_9() {
+    assert_eq!(
+        Query::insert()
+            .into_table(Glyph::Table)
+            .columns([Glyph::Aspect, Glyph::Image])
+            .values_panic([
+                "04108048005887010020060000204E0180400400".into(),
+                3.1415.into(),
+            ])
+            .on_conflict(
+                OnConflict::column(Glyph::Id)
+                    .expr(Func::lower(Expr::col(Glyph::Tokens)))
+                    .update_column(Glyph::Aspect)
+                    .to_owned()
+            )
+            .to_string(PostgresQueryBuilder),
+        [
+            r#"INSERT INTO "glyph" ("aspect", "image")"#,
+            r#"VALUES ('04108048005887010020060000204E0180400400', 3.1415)"#,
+            r#"ON CONFLICT ("id", LOWER("tokens")) DO UPDATE SET "aspect" = "excluded"."aspect""#,
+        ]
+        .join(" ")
+    );
+}
+
 #[test]
 #[allow(clippy::approx_constant)]
 fn insert_returning_all_columns() {