Skip to content

Add value spans to clickbench tests, add docs and From impl #1

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

Closed
wants to merge 5 commits into from
Closed

Add value spans to clickbench tests, add docs and From impl #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4d101b0
Improve documentation and tests, add From impl
alamb Feb 24, 2025
d6116af
Add Expr::value and update clickhouse test
alamb Feb 24, 2025
f0268c9
Fix clippy
alamb Feb 24, 2025
142141a
improve docs
alamb Feb 24, 2025
2f0d1a8
fix
alamb Feb 24, 2025
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
7 changes: 7 additions & 0 deletions src/ast/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1036,6 +1036,13 @@ pub enum Expr {
Lambda(LambdaFunction),
}

impl Expr {
/// Creates a new [`Expr::Value`]
pub fn value(value: impl Into<ValueWithSpan>) -> Self {
Expr::Value(value.into())
}
}

/// The contents inside the `[` and `]` in a subscript expression.
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
Expand Down
51 changes: 48 additions & 3 deletions src/ast/value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,41 @@ use crate::{ast::Ident, tokenizer::Span};
#[cfg(feature = "visitor")]
use sqlparser_derive::{Visit, VisitMut};

/// Primitive SQL values such as number and string
#[derive(Debug, Clone, Eq, Ord)]
/// Wraps a primitive SQL [`Value`] with its [`Span`] location
///
/// # Example: create a `ValueWithSpan` from a `Value`
Copy link
Author

Choose a reason for hiding this comment

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

I added some examples to make the transition easier

/// ```
/// # use sqlparser::ast::{Value, ValueWithSpan};
/// # use sqlparser::tokenizer::{Location, Span};
/// let value = Value::SingleQuotedString(String::from("endpoint"));
/// // from line 1, column 1 to line 1, column 7
/// let span = Span::new(Location::new(1, 1), Location::new(1, 7));
/// let value_with_span = value.with_span(span);
/// ```
///
/// # Example: create a `ValueWithSpan` from a `Value` with an empty span
///
/// You can call [`Value::with_empty_span`] to create a `ValueWithSpan` with an empty span
/// ```
/// # use sqlparser::ast::{Value, ValueWithSpan};
/// # use sqlparser::tokenizer::{Location, Span};
/// let value = Value::SingleQuotedString(String::from("endpoint"));
/// let value_with_span = value.with_empty_span();
/// assert_eq!(value_with_span.span, Span::empty());
/// ```
///
/// You can also use the [`From`] trait to convert `ValueWithSpan` to/from `Value`s
/// ```
/// # use sqlparser::ast::{Value, ValueWithSpan};
/// # use sqlparser::tokenizer::{Location, Span};
/// let value = Value::SingleQuotedString(String::from("endpoint"));
/// // converting `Value` to `ValueWithSpan` results in an empty span
/// let value_with_span: ValueWithSpan = value.into();
/// assert_eq!(value_with_span.span, Span::empty());
/// // convert back to `Value`
/// let value: Value = value_with_span.into();
/// ```
#[derive(Debug, Clone, Eq)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
pub struct ValueWithSpan {
Expand All @@ -47,7 +80,13 @@ impl PartialEq for ValueWithSpan {

impl PartialOrd for ValueWithSpan {
fn partial_cmp(&self, other: &Self) -> Option<core::cmp::Ordering> {
self.value.partial_cmp(&other.value)
Some(self.cmp(other))
}
}

impl Ord for ValueWithSpan {
Copy link
Author

Choose a reason for hiding this comment

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

I did this to fix clippy

fn cmp(&self, other: &Self) -> core::cmp::Ordering {
self.value.cmp(&other.value)
}
}

Expand All @@ -63,6 +102,12 @@ impl From<ValueWithSpan> for Value {
}
}

impl From<Value> for ValueWithSpan {
Copy link
Author

Choose a reason for hiding this comment

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

This makes it easy to create new ValueWithSpan

fn from(value: Value) -> Self {
value.with_empty_span()
}
}

/// Primitive SQL values such as number and string
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
Expand Down
56 changes: 28 additions & 28 deletions tests/sqlparser_clickhouse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ fn parse_map_access_expr() {
"indexOf",
[
Expr::Identifier(Ident::new("string_names")),
Expr::Value(Value::SingleQuotedString("endpoint".to_string()))
Expr::value(Value::SingleQuotedString("endpoint".to_string()))
Copy link
Author

Choose a reason for hiding this comment

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

By adding the Expr::value method, the changes to update the tests became a find/replace exercise

]
),
})],
Expand All @@ -71,7 +71,7 @@ fn parse_map_access_expr() {
left: Box::new(BinaryOp {
left: Box::new(Identifier(Ident::new("id"))),
op: BinaryOperator::Eq,
right: Box::new(Expr::Value(Value::SingleQuotedString("test".to_string()))),
right: Box::new(Expr::value(Value::SingleQuotedString("test".to_string()))),
}),
op: BinaryOperator::And,
right: Box::new(BinaryOp {
Expand All @@ -82,13 +82,13 @@ fn parse_map_access_expr() {
"indexOf",
[
Expr::Identifier(Ident::new("string_name")),
Expr::Value(Value::SingleQuotedString("app".to_string()))
Expr::value(Value::SingleQuotedString("app".to_string()))
]
),
})],
}),
op: BinaryOperator::NotEq,
right: Box::new(Expr::Value(Value::SingleQuotedString("foo".to_string()))),
right: Box::new(Expr::value(Value::SingleQuotedString("foo".to_string()))),
}),
}),
group_by: GroupByExpr::Expressions(vec![], vec![]),
Expand All @@ -114,8 +114,8 @@ fn parse_array_expr() {
assert_eq!(
&Expr::Array(Array {
elem: vec![
Expr::Value(Value::SingleQuotedString("1".to_string())),
Expr::Value(Value::SingleQuotedString("2".to_string())),
Expr::value(Value::SingleQuotedString("1".to_string())),
Expr::value(Value::SingleQuotedString("2".to_string())),
],
named: false,
}),
Expand Down Expand Up @@ -1014,14 +1014,14 @@ fn parse_select_parametric_function() {
assert_eq!(parameters.args.len(), 2);
assert_eq!(
parameters.args[0],
FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Value(Value::Number(
FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::value(Value::Number(
"0.5".parse().unwrap(),
false
))))
);
assert_eq!(
parameters.args[1],
FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Value(Value::Number(
FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::value(Value::Number(
"0.6".parse().unwrap(),
false
))))
Expand Down Expand Up @@ -1074,19 +1074,19 @@ fn parse_select_order_by_with_fill_interpolate() {
asc: Some(true),
nulls_first: Some(true),
with_fill: Some(WithFill {
from: Some(Expr::Value(number("10"))),
to: Some(Expr::Value(number("20"))),
step: Some(Expr::Value(number("2"))),
from: Some(Expr::value(number("10"))),
to: Some(Expr::value(number("20"))),
step: Some(Expr::value(number("2"))),
}),
},
OrderByExpr {
expr: Expr::Identifier(Ident::new("lname")),
asc: Some(false),
nulls_first: Some(false),
with_fill: Some(WithFill {
from: Some(Expr::Value(number("30"))),
to: Some(Expr::Value(number("40"))),
step: Some(Expr::Value(number("3"))),
from: Some(Expr::value(number("30"))),
to: Some(Expr::value(number("40"))),
step: Some(Expr::value(number("3"))),
}),
},
],
Expand All @@ -1096,14 +1096,14 @@ fn parse_select_order_by_with_fill_interpolate() {
expr: Some(Expr::BinaryOp {
left: Box::new(Expr::Identifier(Ident::new("col1"))),
op: BinaryOperator::Plus,
right: Box::new(Expr::Value(number("1"))),
right: Box::new(Expr::value(number("1"))),
}),
}])
})
},
select.order_by.expect("ORDER BY expected")
);
assert_eq!(Some(Expr::Value(number("2"))), select.limit);
assert_eq!(Some(Expr::value(number("2"))), select.limit);
}

#[test]
Expand Down Expand Up @@ -1144,9 +1144,9 @@ fn parse_with_fill() {
let select = clickhouse().verified_query(sql);
assert_eq!(
Some(WithFill {
from: Some(Expr::Value(number("10"))),
to: Some(Expr::Value(number("20"))),
step: Some(Expr::Value(number("2"))),
from: Some(Expr::value(number("10"))),
to: Some(Expr::value(number("20"))),
step: Some(Expr::value(number("2"))),
}),
select.order_by.expect("ORDER BY expected").exprs[0].with_fill
);
Expand Down Expand Up @@ -1183,7 +1183,7 @@ fn parse_interpolate_body_with_columns() {
expr: Some(Expr::BinaryOp {
left: Box::new(Expr::Identifier(Ident::new("col1"))),
op: BinaryOperator::Plus,
right: Box::new(Expr::Value(number("1"))),
right: Box::new(Expr::value(number("1"))),
}),
},
InterpolateExpr {
Expand All @@ -1195,7 +1195,7 @@ fn parse_interpolate_body_with_columns() {
expr: Some(Expr::BinaryOp {
left: Box::new(Expr::Identifier(Ident::new("col4"))),
op: BinaryOperator::Plus,
right: Box::new(Expr::Value(number("4"))),
right: Box::new(Expr::value(number("4"))),
}),
},
])
Expand Down Expand Up @@ -1236,7 +1236,7 @@ fn test_prewhere() {
Some(&BinaryOp {
left: Box::new(Identifier(Ident::new("x"))),
op: BinaryOperator::Eq,
right: Box::new(Expr::Value(Value::Number("1".parse().unwrap(), false))),
right: Box::new(Expr::value(Value::Number("1".parse().unwrap(), false))),
})
);
let selection = query.as_ref().body.as_select().unwrap().selection.as_ref();
Expand All @@ -1245,7 +1245,7 @@ fn test_prewhere() {
Some(&BinaryOp {
left: Box::new(Identifier(Ident::new("y"))),
op: BinaryOperator::Eq,
right: Box::new(Expr::Value(Value::Number("2".parse().unwrap(), false))),
right: Box::new(Expr::value(Value::Number("2".parse().unwrap(), false))),
})
);
}
Expand All @@ -1261,13 +1261,13 @@ fn test_prewhere() {
left: Box::new(BinaryOp {
left: Box::new(Identifier(Ident::new("x"))),
op: BinaryOperator::Eq,
right: Box::new(Expr::Value(Value::Number("1".parse().unwrap(), false))),
right: Box::new(Expr::value(Value::Number("1".parse().unwrap(), false))),
}),
op: BinaryOperator::And,
right: Box::new(BinaryOp {
left: Box::new(Identifier(Ident::new("y"))),
op: BinaryOperator::Eq,
right: Box::new(Expr::Value(Value::Number("2".parse().unwrap(), false))),
right: Box::new(Expr::value(Value::Number("2".parse().unwrap(), false))),
}),
})
);
Expand Down Expand Up @@ -1375,7 +1375,7 @@ fn parse_create_table_on_commit_and_as_query() {
assert_eq!(on_commit, Some(OnCommit::PreserveRows));
assert_eq!(
query.unwrap().body.as_select().unwrap().projection,
vec![UnnamedExpr(Expr::Value(Value::Number(
vec![UnnamedExpr(Expr::value(Value::Number(
"1".parse().unwrap(),
false
)))]
Expand All @@ -1391,7 +1391,7 @@ fn parse_freeze_and_unfreeze_partition() {
for operation_name in &["FREEZE", "UNFREEZE"] {
let sql = format!("ALTER TABLE t {operation_name} PARTITION '2024-08-14'");

let expected_partition = Partition::Expr(Expr::Value(Value::SingleQuotedString(
let expected_partition = Partition::Expr(Expr::value(Value::SingleQuotedString(
"2024-08-14".to_string(),
)));
match clickhouse_and_generic().verified_stmt(&sql) {
Expand Down Expand Up @@ -1421,7 +1421,7 @@ fn parse_freeze_and_unfreeze_partition() {
match clickhouse_and_generic().verified_stmt(&sql) {
Statement::AlterTable { operations, .. } => {
assert_eq!(operations.len(), 1);
let expected_partition = Partition::Expr(Expr::Value(Value::SingleQuotedString(
let expected_partition = Partition::Expr(Expr::value(Value::SingleQuotedString(
"2024-08-14".to_string(),
)));
let expected_operation = if operation_name == &"FREEZE" {
Expand Down