Skip to content

Commit

Permalink
Merge pull request #685 from SeaQL/parentheses_fix
Browse files Browse the repository at this point in the history
Parentheses fix
  • Loading branch information
tyt2y3 authored Aug 25, 2023
2 parents 2b12189 + e993f9a commit 8bafaa0
Show file tree
Hide file tree
Showing 12 changed files with 465 additions and 129 deletions.
109 changes: 109 additions & 0 deletions src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,112 @@ pub trait EscapeBuilder {
output
}
}

pub trait PrecedenceDecider {
// This method decides which precedence relations should lead to dropped parentheses.
// There will be more fine grained precedence relations than the ones represented here,
// but dropping parentheses due to these relations can be confusing for readers.
fn inner_expr_well_known_greater_precedence(
&self,
inner: &SimpleExpr,
outer_oper: &Oper,
) -> bool;
}

pub trait OperLeftAssocDecider {
// This method decides if the left associativity of an operator should lead to dropped parentheses.
// Not all known left associative operators are necessarily included here,
// as dropping them may in some cases be confusing to readers.
fn well_known_left_associative(&self, op: &BinOper) -> bool;
}

#[derive(Debug, PartialEq)]
pub enum Oper {
UnOper(UnOper),
BinOper(BinOper),
}

impl From<UnOper> for Oper {
fn from(value: UnOper) -> Self {
Oper::UnOper(value)
}
}

impl From<BinOper> for Oper {
fn from(value: BinOper) -> Self {
Oper::BinOper(value)
}
}

impl Oper {
pub(crate) fn is_logical(&self) -> bool {
matches!(
self,
Oper::UnOper(UnOper::Not) | Oper::BinOper(BinOper::And) | Oper::BinOper(BinOper::Or)
)
}

pub(crate) fn is_between(&self) -> bool {
matches!(
self,
Oper::BinOper(BinOper::Between) | Oper::BinOper(BinOper::NotBetween)
)
}

pub(crate) fn is_like(&self) -> bool {
matches!(
self,
Oper::BinOper(BinOper::Like) | Oper::BinOper(BinOper::NotLike)
)
}

pub(crate) fn is_in(&self) -> bool {
matches!(
self,
Oper::BinOper(BinOper::In) | Oper::BinOper(BinOper::NotIn)
)
}

pub(crate) fn is_is(&self) -> bool {
matches!(
self,
Oper::BinOper(BinOper::Is) | Oper::BinOper(BinOper::IsNot)
)
}

pub(crate) fn is_shift(&self) -> bool {
matches!(
self,
Oper::BinOper(BinOper::LShift) | Oper::BinOper(BinOper::RShift)
)
}

pub(crate) fn is_arithmetic(&self) -> bool {
match self {
Oper::BinOper(b) => {
matches!(
b,
BinOper::Mul | BinOper::Div | BinOper::Mod | BinOper::Add | BinOper::Sub
)
}
_ => false,
}
}

pub(crate) fn is_comparison(&self) -> bool {
match self {
Oper::BinOper(b) => {
matches!(
b,
BinOper::SmallerThan
| BinOper::SmallerThanOrEqual
| BinOper::Equal
| BinOper::GreaterThanOrEqual
| BinOper::GreaterThan
| BinOper::NotEqual
)
}
_ => false,
}
}
}
16 changes: 16 additions & 0 deletions src/backend/mysql/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,19 @@ impl QuotedBuilder for MysqlQueryBuilder {
impl EscapeBuilder for MysqlQueryBuilder {}

impl TableRefBuilder for MysqlQueryBuilder {}

impl PrecedenceDecider for MysqlQueryBuilder {
fn inner_expr_well_known_greater_precedence(
&self,
inner: &SimpleExpr,
outer_oper: &Oper,
) -> bool {
common_inner_expr_well_known_greater_precedence(inner, outer_oper)
}
}

impl OperLeftAssocDecider for MysqlQueryBuilder {
fn well_known_left_associative(&self, op: &BinOper) -> bool {
common_well_known_left_associative(op)
}
}
51 changes: 51 additions & 0 deletions src/backend/postgres/query.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,38 @@
use super::*;
use crate::extension::postgres::*;

impl OperLeftAssocDecider for PostgresQueryBuilder {
fn well_known_left_associative(&self, op: &BinOper) -> bool {
let common_answer = common_well_known_left_associative(op);
let pg_specific_answer = matches!(op, BinOper::PgOperator(PgBinOper::Concatenate));
common_answer || pg_specific_answer
}
}

impl PrecedenceDecider for PostgresQueryBuilder {
fn inner_expr_well_known_greater_precedence(
&self,
inner: &SimpleExpr,
outer_oper: &Oper,
) -> bool {
let common_answer = common_inner_expr_well_known_greater_precedence(inner, outer_oper);
let pg_specific_answer = match inner {
SimpleExpr::Binary(_, inner_bin_oper, _) => {
let inner_oper: Oper = (*inner_bin_oper).into();
if inner_oper.is_arithmetic() || inner_oper.is_shift() {
is_ilike(inner_bin_oper)
} else if is_pg_comparison(inner_bin_oper) {
outer_oper.is_logical()
} else {
false
}
}
_ => false,
};
common_answer || pg_specific_answer
}
}

impl QueryBuilder for PostgresQueryBuilder {
fn placeholder(&self) -> (&str, bool) {
("$", true)
Expand Down Expand Up @@ -136,3 +168,22 @@ impl QueryBuilder for PostgresQueryBuilder {
"COALESCE"
}
}

fn is_pg_comparison(b: &BinOper) -> bool {
matches!(
b,
BinOper::PgOperator(PgBinOper::Contained)
| BinOper::PgOperator(PgBinOper::Contains)
| BinOper::PgOperator(PgBinOper::Similarity)
| BinOper::PgOperator(PgBinOper::WordSimilarity)
| BinOper::PgOperator(PgBinOper::StrictWordSimilarity)
| BinOper::PgOperator(PgBinOper::Matches)
)
}

fn is_ilike(b: &BinOper) -> bool {
matches!(
b,
BinOper::PgOperator(PgBinOper::ILike) | BinOper::PgOperator(PgBinOper::NotILike)
)
}
Loading

0 comments on commit 8bafaa0

Please sign in to comment.