-
-
Notifications
You must be signed in to change notification settings - Fork 252
IF ELSE statements #828
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
Draft
gronke
wants to merge
2
commits into
SeaQL:master
Choose a base branch
from
gronke:if-else
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
IF ELSE statements #828
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| use crate::{QueryBuilder, SimpleExpr}; | ||
|
|
||
| #[derive(Debug, Clone, PartialEq)] | ||
| pub struct IfElseStatement { | ||
| pub when: SimpleExpr, | ||
| pub then: SimpleExpr, | ||
| pub otherwise: Option<SimpleExpr>, | ||
| } | ||
|
|
||
| impl IfElseStatement { | ||
| pub fn new(when: SimpleExpr, then: SimpleExpr, otherwise: Option<SimpleExpr>) -> Self { | ||
| Self { | ||
| when, | ||
| then, | ||
| otherwise, | ||
| } | ||
| } | ||
|
|
||
| pub fn to_string<T: QueryBuilder>(&self, query_builder: T) -> String { | ||
| let mut sql = String::with_capacity(256); | ||
| query_builder.prepare_if_else_statement(&Box::new(self.clone()), &mut sql); | ||
| sql | ||
| } | ||
| } | ||
| pub trait IfElseStatementBuilder { | ||
| /// Build corresponding SQL statement for certain database backend and return SQL string | ||
| fn build<T: QueryBuilder>(&self, query_builder: T) -> String; | ||
|
|
||
| /// Build corresponding SQL statement for certain database backend and return SQL string | ||
| fn to_string<T: QueryBuilder>(&self, query_builder: T) -> String { | ||
| self.build(query_builder) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| use super::*; | ||
| use pretty_assertions::assert_eq; | ||
|
|
||
| #[rustfmt::skip] | ||
| #[test] | ||
| fn if_without_else() { | ||
| let query = Query::select().column(Asterisk).from(Glyph::Table).take(); | ||
| let then = SimpleExpr::SubQuery(None, Box::new(query.into_sub_query_statement())); | ||
| let if_statement = IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(1), | ||
| then, | ||
| None | ||
| ); | ||
| assert_eq!( | ||
| if_statement.to_string(MysqlQueryBuilder), | ||
| [ | ||
| "IF `id` = 1 THEN", | ||
| "(SELECT * FROM `glyph`)", | ||
| "END IF" | ||
| ].join("\n") | ||
| ) | ||
| } | ||
|
|
||
| #[rustfmt::skip] | ||
| #[test] | ||
| fn if_with_else() { | ||
| let query = Query::select().column(Asterisk).from(Glyph::Table).take(); | ||
| let then = SimpleExpr::SubQuery(None, Box::new(query.into_sub_query_statement())); | ||
| let if_statement = IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(1), | ||
| then, | ||
| Some(Expr::val("23").into()), | ||
| ); | ||
| assert_eq!( | ||
| if_statement.to_string(MysqlQueryBuilder), | ||
| [ | ||
| "IF `id` = 1 THEN", | ||
| "(SELECT * FROM `glyph`)", | ||
| "ELSE", | ||
| "'23'", | ||
| "END IF" | ||
| ] | ||
| .join("\n") | ||
| ) | ||
| } | ||
|
|
||
| #[test] | ||
| fn if_with_elseif() { | ||
| let query = Query::select().column(Asterisk).from(Glyph::Table).take(); | ||
| let then = SimpleExpr::SubQuery(None, Box::new(query.into_sub_query_statement())); | ||
| let if_statement = IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(1), | ||
| then, | ||
| Some(SimpleExpr::IfElse(Box::new(IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(2), | ||
| Expr::val("42").into(), | ||
| None, | ||
| )))), | ||
| ); | ||
| assert_eq!( | ||
| if_statement.to_string(MysqlQueryBuilder), | ||
| [ | ||
| "IF `id` = 1 THEN", | ||
| "(SELECT * FROM `glyph`)", | ||
| "ELSEIF `id` = 2 THEN", | ||
| "'42'", | ||
| "END IF" | ||
| ] | ||
| .join("\n") | ||
| ) | ||
| } | ||
|
|
||
| #[test] | ||
| fn if_with_elseif_and_else() { | ||
| let query = Query::select().column(Asterisk).from(Glyph::Table).take(); | ||
| let then = SimpleExpr::SubQuery(None, Box::new(query.into_sub_query_statement())); | ||
| let if_statement = IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(1), | ||
| then, | ||
| Some(SimpleExpr::IfElse(Box::new(IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(2), | ||
| Expr::val("42").into(), | ||
| Some(Expr::val("9000").into()), | ||
| )))), | ||
| ); | ||
| assert_eq!( | ||
| if_statement.to_string(MysqlQueryBuilder), | ||
| [ | ||
| "IF `id` = 1 THEN", | ||
| "(SELECT * FROM `glyph`)", | ||
| "ELSEIF `id` = 2 THEN", | ||
| "'42'", | ||
| "ELSE", | ||
| "'9000'", | ||
| "END IF" | ||
| ] | ||
| .join("\n") | ||
| ); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| use super::*; | ||
| use pretty_assertions::assert_eq; | ||
|
|
||
| #[test] | ||
| #[rustfmt::skip] | ||
| fn if_without_else() { | ||
| let query = Query::select().column(Asterisk).from(Glyph::Table).take(); | ||
| let then = SimpleExpr::SubQuery(None, Box::new(query.into_sub_query_statement())); | ||
| let if_statement = IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(1), | ||
| then, | ||
| None | ||
| ); | ||
| assert_eq!( | ||
| if_statement.to_string(MysqlQueryBuilder), | ||
| [ | ||
| "IF `id` = 1 THEN", | ||
| "(SELECT * FROM `glyph`)", | ||
| "END IF" | ||
| ].join("\n") | ||
| ) | ||
| } | ||
|
|
||
| #[test] | ||
| #[rustfmt::skip] | ||
| fn if_with_else() { | ||
| let query = Query::select().column(Asterisk).from(Glyph::Table).take(); | ||
| let then = SimpleExpr::SubQuery(None, Box::new(query.into_sub_query_statement())); | ||
| let if_statement = IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(1), | ||
| then, | ||
| Some(Expr::val("23").into()) | ||
| ); | ||
| assert_eq!( | ||
| if_statement.to_string(PostgresQueryBuilder), | ||
| [ | ||
| "IF \"id\" = 1 THEN", | ||
| "(SELECT * FROM \"glyph\")", | ||
| "ELSE", | ||
| "'23'", | ||
| "END IF" | ||
| ].join("\n") | ||
| ) | ||
| } | ||
|
|
||
| #[test] | ||
| #[rustfmt::skip] | ||
| fn if_with_elseif() { | ||
| let query = Query::select().column(Asterisk).from(Glyph::Table).take(); | ||
| let then = SimpleExpr::SubQuery(None, Box::new(query.into_sub_query_statement())); | ||
| let if_statement = IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(1), | ||
| then, | ||
| Some(SimpleExpr::IfElse(Box::new(IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(2), | ||
| Expr::val("123").into(), | ||
| None | ||
| )))) | ||
| ); | ||
| assert_eq!( | ||
| if_statement.to_string(PostgresQueryBuilder), | ||
| [ | ||
| "IF \"id\" = 1 THEN", | ||
| "(SELECT * FROM \"glyph\")", | ||
| "ELSIF \"id\" = 2 THEN", | ||
| "'123'", | ||
| "END IF" | ||
| ].join("\n") | ||
| ) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| use sea_query::{tests_cfg::*, *}; | ||
|
|
||
| mod foreign_key; | ||
| mod if_else; | ||
| mod index; | ||
| mod query; | ||
| mod table; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| #[should_panic] | ||
| #[rustfmt::skip] | ||
| fn if_else_statement_is_unsupported() { | ||
| let if_statement = IfElseStatement::new( | ||
| Expr::col(Glyph::Id).eq(1), | ||
| Expr::val("23").into(), | ||
| None | ||
| ); | ||
| if_statement.to_string(SqliteQueryBuilder); | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this panic. See #829 (comment). Although, I don't know anything about the backend design in
sea_query. Maybe that's just how we do things here.Maybe,
SimpleExprjust shouldn't contain non-portable variants and we should come up with a different way of handlingIfElseStatement.@tyt2y3 @Huliiiiii, do you have any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, our usual approach is to panic when features are not supported.
There are two ways to solve this problem:
IfElseStatementbehind the feature gateThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, it doesn't solve all the issues. It's nice that you won't be able to use if-else if you activate only
sqlitebackend. But if you activate e.g.sqlite+postgresbackends, then this syntax becomes available and you can pass it to thesqlitebackend and hit a runtime panic.Ideally, it should be impossible to pass. But I'm not sure how achievable for us and convenient for the user is that. Someone needs to experiment and document the results.
This is a more achievable route that we could explore and see if it's annoying for the user. Sadly, I don't have time for this right now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could implement backend specific features, closely resembling local specifics. Later then we could build a general IfElseStatement that normalizes the expression of logic and polyfills for each backend.
My personal use case for If-Else statements are triggers. Triggers could be made conditional on SQLite with WHEN clauses by splitting up the one trigger into multiple. Elsewhere our conditions could break down to CASE expressions. Would love to hear whether that would be a general direction to consider.
Meanwhile we could prepare such step by building backend specific features that only later become available globally. It would be nice to avoid panicking, but it would also be possible to solve that later by introducing a polyfill.