From 1914d72b0096326244b9d1f94fad073a753a29c5 Mon Sep 17 00:00:00 2001 From: Petr Novotnik Date: Mon, 1 Dec 2025 18:03:23 +0100 Subject: [PATCH 1/3] [Oracle] Lower StringConcat precedence --- src/ast/query.rs | 9 +++++ src/dialect/oracle.rs | 38 +++++++++++++++++- tests/sqlparser_oracle.rs | 81 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 tests/sqlparser_oracle.rs diff --git a/src/ast/query.rs b/src/ast/query.rs index 16fc9ec0e..67972f256 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -174,6 +174,15 @@ impl SetExpr { None } } + + /// If this `SetExpr` is a `SELECT`, returns a mutable [`Select`]. + pub fn as_select_mut(&mut self) -> Option<&mut Select> { + if let Self::Select(select) = self { + Some(&mut **select) + } else { + None + } + } } impl fmt::Display for SetExpr { diff --git a/src/dialect/oracle.rs b/src/dialect/oracle.rs index 0d6aee5e6..10387d637 100644 --- a/src/dialect/oracle.rs +++ b/src/dialect/oracle.rs @@ -15,7 +15,14 @@ // specific language governing permissions and limitations // under the License. -use super::Dialect; +use log::debug; + +use crate::{ + parser::{Parser, ParserError}, + tokenizer::Token, +}; + +use super::{Dialect, Precedence}; /// A [`Dialect`] for [Oracle Databases](https://docs.oracle.com/en/database/oracle/oracle-database/21/sqlrf/index.html) #[derive(Debug)] @@ -75,6 +82,35 @@ impl Dialect for OracleDialect { true } + fn get_next_precedence(&self, _parser: &Parser) -> Option> { + let t = _parser.peek_token(); + debug!("get_next_precedence() {t:?}"); + + match t.token { + Token::StringConcat => Some(Ok(self.prec_value(Precedence::PlusMinus))), + _ => None, + } + } + + fn prec_value(&self, prec: Precedence) -> u8 { + match prec { + Precedence::Period => 100, + Precedence::DoubleColon => 50, + Precedence::AtTz => 41, + Precedence::MulDivModOp => 40, + Precedence::PlusMinus => 30, + Precedence::Xor => 24, + Precedence::Ampersand => 23, + Precedence::Caret => 22, + Precedence::Pipe => 21, + Precedence::Between | Precedence::Eq | Precedence::Like | Precedence::Is => 20, + Precedence::PgOther => 16, + Precedence::UnaryNot => 15, + Precedence::And => 10, + Precedence::Or => 5, + } + } + fn supports_group_by_expr(&self) -> bool { true } diff --git a/tests/sqlparser_oracle.rs b/tests/sqlparser_oracle.rs new file mode 100644 index 000000000..d9a3357f5 --- /dev/null +++ b/tests/sqlparser_oracle.rs @@ -0,0 +1,81 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#![warn(clippy::all)] +//! Test SQL syntax, specific to [sqlparser::dialect::OracleDialect]. + +extern crate core; + +#[cfg(test)] +use pretty_assertions::assert_eq; + +use sqlparser::{ + ast::{Expr, SelectItem, Value}, + dialect::OracleDialect, +}; +#[cfg(test)] +use test_utils::TestedDialects; + +mod test_utils; + +#[test] +fn muldiv_have_higher_precedence_than_strconcat() { + // ~ oracle: `||` has a lower precedence than `*` and `/` + let sql = "SELECT 3 / 5 || 'asdf' || 7 * 9 FROM dual"; + let mut query = oracle_dialect().verified_query(sql); + nest_binary_ops(&mut query.body.as_select_mut().expect("not a SELECT").projection[0]); + assert_eq!( + &format!("{query}"), + "SELECT (((3 / 5) || 'asdf') || (7 * 9)) FROM dual" + ); +} + +#[test] +fn plusminus_have_same_precedence_as_strconcat() { + // ~ oracle: `+`, `-`, and `||` have the same precedence and parse from left-to-right + let sql = "SELECT 3 + 5 || '.3' || 7 - 9 FROM dual"; + let mut query = oracle_dialect().verified_query(sql); + nest_binary_ops(&mut query.body.as_select_mut().expect("not a SELECT").projection[0]); + assert_eq!( + &format!("{query}"), + "SELECT ((((3 + 5) || '.3') || 7) - 9) FROM dual" + ); +} + +fn oracle_dialect() -> TestedDialects { + TestedDialects::new(vec![Box::new(OracleDialect)]) +} + +/// Wraps [Expr::BinaryExpr]s in `item` with a [Expr::Nested] recursively. +fn nest_binary_ops(item: &mut SelectItem) { + // ~ idealy, we could use `VisitorMut` at this point + fn nest(expr: &mut Expr) { + // ~ ideally we could use VisitorMut here + if let Expr::BinaryOp { left, op: _, right } = expr { + nest(&mut *left); + nest(&mut *right); + let inner = std::mem::replace(expr, Expr::Value(Value::Null.into())); + *expr = Expr::Nested(Box::new(inner)); + } + } + match item { + SelectItem::UnnamedExpr(expr) => nest(expr), + SelectItem::ExprWithAlias { expr, alias: _ } => nest(expr), + SelectItem::QualifiedWildcard(_, _) => {} + SelectItem::Wildcard(_) => {} + } +} From 2c77d9dcf7fc45d40265903aa66d2d0f8a4d83e7 Mon Sep 17 00:00:00 2001 From: Petr Novotnik Date: Mon, 8 Dec 2025 14:25:09 +0100 Subject: [PATCH 2/3] Avoid helper methods --- src/ast/query.rs | 9 ---- tests/sqlparser_oracle.rs | 104 +++++++++++++++++++++++--------------- 2 files changed, 64 insertions(+), 49 deletions(-) diff --git a/src/ast/query.rs b/src/ast/query.rs index 67972f256..16fc9ec0e 100644 --- a/src/ast/query.rs +++ b/src/ast/query.rs @@ -174,15 +174,6 @@ impl SetExpr { None } } - - /// If this `SetExpr` is a `SELECT`, returns a mutable [`Select`]. - pub fn as_select_mut(&mut self) -> Option<&mut Select> { - if let Self::Select(select) = self { - Some(&mut **select) - } else { - None - } - } } impl fmt::Display for SetExpr { diff --git a/tests/sqlparser_oracle.rs b/tests/sqlparser_oracle.rs index d9a3357f5..09fd41912 100644 --- a/tests/sqlparser_oracle.rs +++ b/tests/sqlparser_oracle.rs @@ -15,67 +15,91 @@ // specific language governing permissions and limitations // under the License. -#![warn(clippy::all)] //! Test SQL syntax, specific to [sqlparser::dialect::OracleDialect]. -extern crate core; - #[cfg(test)] use pretty_assertions::assert_eq; use sqlparser::{ - ast::{Expr, SelectItem, Value}, + ast::{BinaryOperator, Expr, Value, ValueWithSpan}, dialect::OracleDialect, + tokenizer::Span, }; -#[cfg(test)] -use test_utils::TestedDialects; +use test_utils::{expr_from_projection, number, TestedDialects}; mod test_utils; +fn oracle() -> TestedDialects { + TestedDialects::new(vec![Box::new(OracleDialect)]) +} + +/// Oracle: `||` has a lower precedence than `*` and `/` #[test] fn muldiv_have_higher_precedence_than_strconcat() { - // ~ oracle: `||` has a lower precedence than `*` and `/` + // ............... A .. B ...... C .. D ........... let sql = "SELECT 3 / 5 || 'asdf' || 7 * 9 FROM dual"; - let mut query = oracle_dialect().verified_query(sql); - nest_binary_ops(&mut query.body.as_select_mut().expect("not a SELECT").projection[0]); + let select = oracle().verified_only_select(sql); + assert_eq!(1, select.projection.len()); assert_eq!( - &format!("{query}"), - "SELECT (((3 / 5) || 'asdf') || (7 * 9)) FROM dual" + expr_from_projection(&select.projection[0]), + // (C || D) + &Expr::BinaryOp { + // (A || B) + left: Box::new(Expr::BinaryOp { + // A + left: Box::new(Expr::BinaryOp { + left: Box::new(Expr::Value(number("3").into())), + op: BinaryOperator::Divide, + right: Box::new(Expr::Value(number("5").into())), + }), + op: BinaryOperator::StringConcat, + right: Box::new(Expr::Value(ValueWithSpan { + value: Value::SingleQuotedString("asdf".into()), + span: Span::empty(), + })), + }), + op: BinaryOperator::StringConcat, + // D + right: Box::new(Expr::BinaryOp { + left: Box::new(Expr::Value(number("7").into())), + op: BinaryOperator::Multiply, + right: Box::new(Expr::Value(number("9").into())), + }), + } ); } +/// Oracle: `+`, `-`, and `||` have the same precedence and parse from left-to-right #[test] fn plusminus_have_same_precedence_as_strconcat() { - // ~ oracle: `+`, `-`, and `||` have the same precedence and parse from left-to-right + // ................ A .. B .... C .. D ............ let sql = "SELECT 3 + 5 || '.3' || 7 - 9 FROM dual"; - let mut query = oracle_dialect().verified_query(sql); - nest_binary_ops(&mut query.body.as_select_mut().expect("not a SELECT").projection[0]); + let select = oracle().verified_only_select(sql); + assert_eq!(1, select.projection.len()); assert_eq!( - &format!("{query}"), - "SELECT ((((3 + 5) || '.3') || 7) - 9) FROM dual" - ); -} - -fn oracle_dialect() -> TestedDialects { - TestedDialects::new(vec![Box::new(OracleDialect)]) -} - -/// Wraps [Expr::BinaryExpr]s in `item` with a [Expr::Nested] recursively. -fn nest_binary_ops(item: &mut SelectItem) { - // ~ idealy, we could use `VisitorMut` at this point - fn nest(expr: &mut Expr) { - // ~ ideally we could use VisitorMut here - if let Expr::BinaryOp { left, op: _, right } = expr { - nest(&mut *left); - nest(&mut *right); - let inner = std::mem::replace(expr, Expr::Value(Value::Null.into())); - *expr = Expr::Nested(Box::new(inner)); + expr_from_projection(&select.projection[0]), + // D + &Expr::BinaryOp { + left: Box::new(Expr::BinaryOp { + // B + left: Box::new(Expr::BinaryOp { + // A + left: Box::new(Expr::BinaryOp { + left: Box::new(Expr::Value(number("3").into())), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(number("5").into())), + }), + op: BinaryOperator::StringConcat, + right: Box::new(Expr::Value(ValueWithSpan { + value: Value::SingleQuotedString(".3".into()), + span: Span::empty(), + })), + }), + op: BinaryOperator::StringConcat, + right: Box::new(Expr::Value(number("7").into())), + }), + op: BinaryOperator::Minus, + right: Box::new(Expr::Value(number("9").into())) } - } - match item { - SelectItem::UnnamedExpr(expr) => nest(expr), - SelectItem::ExprWithAlias { expr, alias: _ } => nest(expr), - SelectItem::QualifiedWildcard(_, _) => {} - SelectItem::Wildcard(_) => {} - } + ); } From af34d0c9ed2f109d264c4756989cde492f2dfd9d Mon Sep 17 00:00:00 2001 From: Petr Novotnik Date: Mon, 8 Dec 2025 14:48:29 +0100 Subject: [PATCH 3/3] Remove precedence overrides --- src/dialect/oracle.rs | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/src/dialect/oracle.rs b/src/dialect/oracle.rs index 10387d637..f8bb0e155 100644 --- a/src/dialect/oracle.rs +++ b/src/dialect/oracle.rs @@ -82,8 +82,8 @@ impl Dialect for OracleDialect { true } - fn get_next_precedence(&self, _parser: &Parser) -> Option> { - let t = _parser.peek_token(); + fn get_next_precedence(&self, parser: &Parser) -> Option> { + let t = parser.peek_token(); debug!("get_next_precedence() {t:?}"); match t.token { @@ -92,25 +92,6 @@ impl Dialect for OracleDialect { } } - fn prec_value(&self, prec: Precedence) -> u8 { - match prec { - Precedence::Period => 100, - Precedence::DoubleColon => 50, - Precedence::AtTz => 41, - Precedence::MulDivModOp => 40, - Precedence::PlusMinus => 30, - Precedence::Xor => 24, - Precedence::Ampersand => 23, - Precedence::Caret => 22, - Precedence::Pipe => 21, - Precedence::Between | Precedence::Eq | Precedence::Like | Precedence::Is => 20, - Precedence::PgOther => 16, - Precedence::UnaryNot => 15, - Precedence::And => 10, - Precedence::Or => 5, - } - } - fn supports_group_by_expr(&self) -> bool { true }