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.

Sign up for GitHub

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

Jump to bottom

gh-516: support coexistence of SimpleExpr and QueryStatement #572

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

gh-516: support coexistence of SimpleExpr and QueryStatement #572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f8a2cfa
gh-516: support coexistence of SimpleExpr and QueryStatement
alphavector Jan 2, 2023
6dc6fc9
gh-516: fix formatting
alphavector Jan 3, 2023
bdd64a0
gh-516: call `.take()` instead of `.to_owned()`
alphavector Feb 25, 2023
e5f0b64
gh-516: use into_sub_query_expr instead of From
alphavector Apr 27, 2023
e88fc4f
gh-516: minor fix
alphavector Apr 27, 2023
2e24c0c
Merge remote-tracking branch 'upstream/master' into feature/coalesce_…
alphavector Apr 27, 2023
0c79402
gh-516: reorder the tests
alphavector Apr 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ pub enum SimpleExpr {
Constant(Value),
}

pub trait IntoSimpleExpr {
fn into_sub_query_expr(self) -> SimpleExpr;
Copy link
Member

@ikrivosheev ikrivosheev Jul 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@alphavector hello! Naming is confusing me a little bit)

@tyt2y3 what do you think?

pub trait IntoSimpleExpr {
   fn into_simple_expr(self) -> SimpleExpr;
}

Copy link
Member

Choose a reason for hiding this comment

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

Actually, there is only one impl for this trait. May be we can simply make it an inherit method?

}

pub(crate) mod private {
use crate::{BinOper, LikeExpr, SimpleExpr, UnOper};

Expand Down
12 changes: 8 additions & 4 deletions src/func.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,11 @@ impl Func {
///
/// let query = Query::select()
/// .expr(Func::coalesce([
/// Expr::col(Char::SizeW).into(),
/// Query::select()
/// .from(Char::Table)
/// .expr(Func::max(Expr::col(Char::SizeW)))
/// .take()
/// .into_sub_query_expr(),
/// Expr::col(Char::SizeH).into(),
/// Expr::val(12).into(),
/// ]))
Expand All @@ -418,15 +422,15 @@ impl Func {
///
/// assert_eq!(
/// query.to_string(MysqlQueryBuilder),
/// r#"SELECT COALESCE(`size_w`, `size_h`, 12) FROM `character`"#
/// r#"SELECT COALESCE((SELECT MAX(`size_w`) FROM `character`), `size_h`, 12) FROM `character`"#
Copy link
Member

Choose a reason for hiding this comment

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

And btw, can you add a new test case instead? For better test coverage.

Copy link
Author

Choose a reason for hiding this comment

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

What do you mean by a new test case? I added test cases, insert_coalesce for example. The current test case involves checking for a subquery in coalesce

/// );
/// assert_eq!(
/// query.to_string(PostgresQueryBuilder),
/// r#"SELECT COALESCE("size_w", "size_h", 12) FROM "character""#
/// r#"SELECT COALESCE((SELECT MAX("size_w") FROM "character"), "size_h", 12) FROM "character""#
/// );
/// assert_eq!(
/// query.to_string(SqliteQueryBuilder),
/// r#"SELECT COALESCE("size_w", "size_h", 12) FROM "character""#
/// r#"SELECT COALESCE((SELECT MAX("size_w") FROM "character"), "size_h", 12) FROM "character""#
/// );
/// ```
pub fn coalesce<I>(args: I) -> FunctionCall
Expand Down
6 changes: 6 additions & 0 deletions src/query/select.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2380,3 +2380,9 @@ impl ConditionalStatement for SelectStatement {
pub fn and_where_option(&mut self, other: Option<SimpleExpr>) -> &mut Self;
pub fn and_where(&mut self, other: SimpleExpr) -> &mut Self;
}

impl IntoSimpleExpr for SelectStatement {
fn into_sub_query_expr(self) -> SimpleExpr {
SimpleExpr::SubQuery(None, Box::new(self.into_sub_query_statement()))
}
}
47 changes: 47 additions & 0 deletions tests/mysql/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,27 @@ fn select_58() {
);
}

#[test]
fn select_coalesce() {
let query = Query::select()
.expr(Func::coalesce([
Query::select()
.from(Char::Table)
.expr(Func::max(Expr::col(Character::Id)))
.take()
.into_sub_query_expr(),
1.into(),
Value::Bool(None).into(),
]))
.from(Char::Table)
.to_owned();

assert_eq!(
query.to_string(MysqlQueryBuilder),
r#"SELECT COALESCE((SELECT MAX(`id`) FROM `character`), 1, NULL) FROM `character`"#
);
}

#[test]
#[allow(clippy::approx_constant)]
fn insert_2() {
Expand Down Expand Up @@ -1328,6 +1349,32 @@ fn insert_on_conflict_6() {
);
}

#[test]
fn insert_coalesce() {
assert_eq!(Query::insert()
.into_table(Glyph::Table)
.columns([Glyph::Image, Glyph::Aspect])
.values_panic([
"04108048005887010020060000204E0180400400".into(),
Func::coalesce([Query::select()
.from(Glyph::Table)
.expr(Func::max(Expr::col(Glyph::Aspect)))
.take()
.into_sub_query_expr(),
1.into(),
Value::Bool(None).into(),
])
.into(),
])
.to_string(MysqlQueryBuilder),
[
r#"INSERT INTO `glyph` (`image`, `aspect`)"#,
r#"VALUES ('04108048005887010020060000204E0180400400', COALESCE((SELECT MAX(`aspect`) FROM `glyph`), 1, NULL))"#,
]
.join(" ")
);
}

#[test]
fn update_1() {
assert_eq!(
Expand Down
50 changes: 49 additions & 1 deletion tests/postgres/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::*;
use pretty_assertions::assert_eq;
use sea_query::extension::postgres::PgBinOper;
use sea_query::Value;

#[test]
fn select_1() {
Expand Down Expand Up @@ -103,7 +104,7 @@ fn select_8() {
.from(Char::Table)
.left_join(
Font::Table,
Expr::col((Char::Table, Char::FontId)).equals((Font::Table, Font::Id))
Expr::col((Char::Table, Char::FontId)).equals((Font::Table, Font::Id)),
)
.to_string(PostgresQueryBuilder),
r#"SELECT "character" FROM "character" LEFT JOIN "font" ON "character"."font_id" = "font"."id""#
Expand Down Expand Up @@ -1095,6 +1096,27 @@ fn select_62() {
);
}

#[test]
fn select_coalesce() {
let query = Query::select()
.expr(Func::coalesce([
Query::select()
.from(Char::Table)
.expr(Func::count(Expr::col(Character::Id)))
.take()
.into_sub_query_expr(),
1.into(),
Value::Bool(None).into(),
]))
.from(Char::Table)
.to_owned();

assert_eq!(
query.to_string(PostgresQueryBuilder),
r#"SELECT COALESCE((SELECT COUNT("id") FROM "character"), 1, NULL) FROM "character""#
);
}

#[test]
#[allow(clippy::approx_constant)]
fn insert_2() {
Expand Down Expand Up @@ -1473,6 +1495,32 @@ fn insert_returning_specific_columns() {
);
}

#[test]
fn insert_coalesce() {
assert_eq!(Query::insert()
.into_table(Glyph::Table)
.columns([Glyph::Image, Glyph::Aspect])
.values_panic([
"04108048005887010020060000204E0180400400".into(),
Func::coalesce([Query::select()
.from(Glyph::Table)
.expr(Func::max(Expr::col(Glyph::Aspect)))
.take()
.into_sub_query_expr(),
1.into(),
Value::Bool(None).into(),
])
.into(),
])
.to_string(PostgresQueryBuilder),
[
r#"INSERT INTO "glyph" ("image", "aspect")"#,
r#"VALUES ('04108048005887010020060000204E0180400400', COALESCE((SELECT MAX("aspect") FROM "glyph"), 1, NULL))"#,
]
.join(" ")
);
}

#[test]
fn update_1() {
assert_eq!(
Expand Down
47 changes: 47 additions & 0 deletions tests/sqlite/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,27 @@ fn cast_json_field_bin_oper() {
);
}

#[test]
fn select_coalesce() {
let query = Query::select()
.expr(Func::coalesce([
Query::select()
.from(Char::Table)
.expr(Func::max(Expr::col(Character::Id)))
.take()
.into_sub_query_expr(),
1.into(),
Value::Bool(None).into(),
]))
.from(Char::Table)
.to_owned();

assert_eq!(
query.to_string(SqliteQueryBuilder),
r#"SELECT COALESCE((SELECT MAX("id") FROM "character"), 1, NULL) FROM "character""#
);
}

#[test]
#[allow(clippy::approx_constant)]
fn insert_2() {
Expand Down Expand Up @@ -1389,6 +1410,32 @@ fn insert_returning_specific_columns() {
);
}

#[test]
fn insert_coalesce() {
assert_eq!(Query::insert()
.into_table(Glyph::Table)
.columns([Glyph::Image, Glyph::Aspect])
.values_panic([
"04108048005887010020060000204E0180400400".into(),
Func::coalesce([Query::select()
.from(Glyph::Table)
.expr(Func::max(Expr::col(Glyph::Aspect)))
.take()
.into_sub_query_expr(),
1.into(),
Value::Bool(None).into(),
])
.into(),
])
.to_string(SqliteQueryBuilder),
[
r#"INSERT INTO "glyph" ("image", "aspect")"#,
r#"VALUES ('04108048005887010020060000204E0180400400', COALESCE((SELECT MAX("aspect") FROM "glyph"), 1, NULL))"#,
]
.join(" ")
);
}

#[test]
fn update_1() {
assert_eq!(
Expand Down