Skip to content

Commit

Permalink
fix: support unary operator in default value, partition rule and prep…
Browse files Browse the repository at this point in the history
…are statement (#4301)

* handle unary operator

Signed-off-by: Ruihang Xia <[email protected]>

* add sqlness test

Signed-off-by: Ruihang Xia <[email protected]>

* add prepare test

Signed-off-by: Ruihang Xia <[email protected]>

* add test and context

Signed-off-by: Ruihang Xia <[email protected]>

* fix rebase error

Signed-off-by: Ruihang Xia <[email protected]>

* fix merge error

Signed-off-by: Ruihang Xia <[email protected]>

* fix sqlness

Signed-off-by: Ruihang Xia <[email protected]>

---------

Signed-off-by: Ruihang Xia <[email protected]>
Co-authored-by: dennis zhuang <[email protected]>
  • Loading branch information
waynexia and killme2008 authored Jul 9, 2024
1 parent 7fe3f49 commit 185953e
Show file tree
Hide file tree
Showing 21 changed files with 422 additions and 35 deletions.
5 changes: 5 additions & 0 deletions src/common/decimal/src/decimal128.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ impl Decimal128 {
let value = (hi | lo) as i128;
Self::new(value, precision, scale)
}

pub fn negative(mut self) -> Self {
self.value = -self.value;
self
}
}

/// The default value of Decimal128 is 0, and its precision is 1 and scale is 0.
Expand Down
4 changes: 4 additions & 0 deletions src/common/time/src/date.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ impl Date {
.checked_sub_days(Days::new(days as u64))
.map(Into::into)
}

pub fn negative(&self) -> Self {
Self(-self.0)
}
}

#[cfg(test)]
Expand Down
4 changes: 4 additions & 0 deletions src/common/time/src/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ impl DateTime {
pub fn to_date(&self) -> Option<Date> {
self.to_chrono_datetime().map(|d| Date::from(d.date()))
}

pub fn negative(&self) -> Self {
Self(-self.0)
}
}

#[cfg(test)]
Expand Down
5 changes: 5 additions & 0 deletions src/common/time/src/duration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ impl Duration {
pub fn to_std_duration(self) -> std::time::Duration {
self.into()
}

pub fn negative(mut self) -> Self {
self.value = -self.value;
self
}
}

/// Convert i64 to Duration Type.
Expand Down
9 changes: 9 additions & 0 deletions src/common/time/src/interval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,15 @@ impl Interval {
pub fn to_i32(&self) -> i32 {
self.months
}

pub fn negative(&self) -> Self {
Self {
months: -self.months,
days: -self.days,
nsecs: -self.nsecs,
unit: self.unit,
}
}
}

impl From<i128> for Interval {
Expand Down
5 changes: 5 additions & 0 deletions src/common/time/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ impl Time {
None
}
}

pub fn negative(mut self) -> Self {
self.value = -self.value;
self
}
}

impl From<i64> for Time {
Expand Down
5 changes: 5 additions & 0 deletions src/common/time/src/timestamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,11 @@ impl Timestamp {

ParseTimestampSnafu { raw: s }.fail()
}

pub fn negative(mut self) -> Self {
self.value = -self.value;
self
}
}

impl Timestamp {
Expand Down
50 changes: 50 additions & 0 deletions src/datatypes/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,56 @@ impl Value {

Ok(scalar_value)
}

/// Apply `-` unary op if possible
pub fn try_negative(&self) -> Option<Self> {
match self {
Value::Null => Some(Value::Null),
Value::UInt8(x) => {
if *x == 0 {
Some(Value::UInt8(*x))
} else {
None
}
}
Value::UInt16(x) => {
if *x == 0 {
Some(Value::UInt16(*x))
} else {
None
}
}
Value::UInt32(x) => {
if *x == 0 {
Some(Value::UInt32(*x))
} else {
None
}
}
Value::UInt64(x) => {
if *x == 0 {
Some(Value::UInt64(*x))
} else {
None
}
}
Value::Int8(x) => Some(Value::Int8(-*x)),
Value::Int16(x) => Some(Value::Int16(-*x)),
Value::Int32(x) => Some(Value::Int32(-*x)),
Value::Int64(x) => Some(Value::Int64(-*x)),
Value::Float32(x) => Some(Value::Float32(-*x)),
Value::Float64(x) => Some(Value::Float64(-*x)),
Value::Decimal128(x) => Some(Value::Decimal128(x.negative())),
Value::Date(x) => Some(Value::Date(x.negative())),
Value::DateTime(x) => Some(Value::DateTime(x.negative())),
Value::Timestamp(x) => Some(Value::Timestamp(x.negative())),
Value::Time(x) => Some(Value::Time(x.negative())),
Value::Duration(x) => Some(Value::Duration(x.negative())),
Value::Interval(x) => Some(Value::Interval(x.negative())),

Value::Binary(_) | Value::String(_) | Value::Boolean(_) | Value::List(_) => None,
}
}
}

pub trait TryAsPrimitive<T: LogicalPrimitiveType> {
Expand Down
1 change: 1 addition & 0 deletions src/operator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

#![feature(assert_matches)]
#![feature(if_let_guard)]

pub mod delete;
pub mod error;
Expand Down
2 changes: 1 addition & 1 deletion src/operator/src/req_convert/insert/stmt_to_region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ fn sql_value_to_grpc_value(
column: column.clone(),
})?
} else {
statements::sql_value_to_value(column, &column_schema.data_type, sql_val, timezone)
statements::sql_value_to_value(column, &column_schema.data_type, sql_val, timezone, None)
.context(ParseSqlSnafu)?
};

Expand Down
26 changes: 22 additions & 4 deletions src/operator/src/statement/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use sql::statements::create::{
};
use sql::statements::sql_value_to_value;
use sql::statements::statement::Statement;
use sqlparser::ast::{Expr, Ident, Value as ParserValue};
use sqlparser::ast::{Expr, Ident, UnaryOperator, Value as ParserValue};
use store_api::metric_engine_consts::{LOGICAL_TABLE_METADATA_KEY, METRIC_ENGINE_NAME};
use substrait::{DFLogicalSubstraitConvertor, SubstraitPlan};
use table::dist_table::DistTable;
Expand Down Expand Up @@ -1329,14 +1329,30 @@ fn convert_one_expr(

// convert leaf node.
let (lhs, op, rhs) = match (left.as_ref(), right.as_ref()) {
// col, val
(Expr::Identifier(ident), Expr::Value(value)) => {
let (column_name, data_type) = convert_identifier(ident, column_name_and_type)?;
let value = convert_value(value, data_type, timezone)?;
let value = convert_value(value, data_type, timezone, None)?;
(Operand::Column(column_name), op, Operand::Value(value))
}
(Expr::Identifier(ident), Expr::UnaryOp { op: unary_op, expr })
if let Expr::Value(v) = &**expr =>
{
let (column_name, data_type) = convert_identifier(ident, column_name_and_type)?;
let value = convert_value(v, data_type, timezone, Some(*unary_op))?;
(Operand::Column(column_name), op, Operand::Value(value))
}
// val, col
(Expr::Value(value), Expr::Identifier(ident)) => {
let (column_name, data_type) = convert_identifier(ident, column_name_and_type)?;
let value = convert_value(value, data_type, timezone)?;
let value = convert_value(value, data_type, timezone, None)?;
(Operand::Value(value), op, Operand::Column(column_name))
}
(Expr::UnaryOp { op: unary_op, expr }, Expr::Identifier(ident))
if let Expr::Value(v) = &**expr =>
{
let (column_name, data_type) = convert_identifier(ident, column_name_and_type)?;
let value = convert_value(v, data_type, timezone, Some(*unary_op))?;
(Operand::Value(value), op, Operand::Column(column_name))
}
(Expr::BinaryOp { .. }, Expr::BinaryOp { .. }) => {
Expand Down Expand Up @@ -1372,8 +1388,10 @@ fn convert_value(
value: &ParserValue,
data_type: ConcreteDataType,
timezone: &Timezone,
unary_op: Option<UnaryOperator>,
) -> Result<Value> {
sql_value_to_value("<NONAME>", &data_type, value, Some(timezone)).context(ParseSqlValueSnafu)
sql_value_to_value("<NONAME>", &data_type, value, Some(timezone), unary_op)
.context(ParseSqlValueSnafu)
}

/// Merge table level table options with schema level table options.
Expand Down
1 change: 1 addition & 0 deletions src/servers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#![feature(try_blocks)]
#![feature(exclusive_wrapper)]
#![feature(let_chains)]
#![feature(if_let_guard)]

use datatypes::schema::Schema;
use query::plan::LogicalPlan;
Expand Down
14 changes: 13 additions & 1 deletion src/servers/src/mysql/helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,19 @@ pub fn convert_value(param: &ParamValue, t: &ConcreteDataType) -> Result<ScalarV
pub fn convert_expr_to_scalar_value(param: &Expr, t: &ConcreteDataType) -> Result<ScalarValue> {
match param {
Expr::Value(v) => {
let v = sql_value_to_value("", t, v, None);
let v = sql_value_to_value("", t, v, None, None);
match v {
Ok(v) => v
.try_to_scalar_value(t)
.context(error::ConvertScalarValueSnafu),
Err(e) => error::InvalidParameterSnafu {
reason: e.to_string(),
}
.fail(),
}
}
Expr::UnaryOp { op, expr } if let Expr::Value(v) = &**expr => {
let v = sql_value_to_value("", t, v, None, Some(*op));
match v {
Ok(v) => v
.try_to_scalar_value(t)
Expand Down
20 changes: 19 additions & 1 deletion src/sql/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use common_macro::stack_trace_debug;
use common_time::timestamp::TimeUnit;
use common_time::Timestamp;
use datafusion_common::DataFusionError;
use datafusion_sql::sqlparser::ast::UnaryOperator;
use datatypes::prelude::{ConcreteDataType, Value};
use snafu::{Location, Snafu};
use sqlparser::ast::Ident;
Expand Down Expand Up @@ -161,6 +162,21 @@ pub enum Error {
source: datatypes::error::Error,
},

#[snafu(display("Invalid unary operator {} for value {}", unary_op, value))]
InvalidUnaryOp {
unary_op: UnaryOperator,
value: Value,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Unsupported unary operator {}", unary_op))]
UnsupportedUnaryOp {
unary_op: UnaryOperator,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Unrecognized table option key: {}", key))]
InvalidTableOption {
key: String,
Expand Down Expand Up @@ -299,14 +315,16 @@ impl ErrorExt for Error {
| ConvertToLogicalExpression { .. }
| Simplification { .. }
| InvalidInterval { .. }
| PermissionDenied { .. }
| InvalidUnaryOp { .. }
| UnsupportedUnaryOp { .. }
| FulltextInvalidOption { .. } => StatusCode::InvalidArguments,

SerializeColumnDefaultConstraint { source, .. } => source.status_code(),
ConvertToGrpcDataType { source, .. } => source.status_code(),
ConvertToDfStatement { .. } => StatusCode::Internal,
ConvertSqlValue { .. } | ConvertValue { .. } => StatusCode::Unsupported,

PermissionDenied { .. } => StatusCode::PermissionDenied,
SetFulltextOption { .. } => StatusCode::Unexpected,
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/sql/src/parsers/create_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,10 @@ fn ensure_one_expr(expr: &Expr, columns: &[&Column]) -> Result<()> {
Ok(())
}
Expr::Value(_) => Ok(()),
Expr::UnaryOp { expr, .. } => {
ensure_one_expr(expr, columns)?;
Ok(())
}
_ => error::InvalidSqlSnafu {
msg: format!("Partition rule expr {:?} is not a binary expr!", expr),
}
Expand Down
Loading

0 comments on commit 185953e

Please sign in to comment.