Skip to content
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

Upgrade to sqlparser 0.53.0 #13767

Merged
merged 7 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ recursive = "0.1.1"
regex = "1.8"
rstest = "0.23.0"
serde_json = "1"
sqlparser = { version = "0.52.0", features = ["visitor"] }
sqlparser = { version = "0.53.0", features = ["visitor"] }
tempfile = "3"
tokio = { version = "1.36", features = ["macros", "rt", "sync"] }
url = "2.2"
Expand Down
8 changes: 4 additions & 4 deletions datafusion-cli/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions datafusion/common/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -887,10 +887,10 @@ pub fn get_available_parallelism() -> usize {

#[cfg(test)]
mod tests {
use super::*;
use crate::ScalarValue::Null;
use arrow::array::Float64Array;

use super::*;
use sqlparser::tokenizer::Span;

#[test]
fn test_bisect_linear_left_and_right() -> Result<()> {
Expand Down Expand Up @@ -1118,6 +1118,7 @@ mod tests {
let expected_parsed = vec![Ident {
value: identifier.to_string(),
quote_style,
span: Span::empty(),
}];

assert_eq!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ use arrow_array::{
Array, ArrayRef, Float32Array, Float64Array, Int32Array, RecordBatch, StringArray,
};
use arrow_schema::{DataType, Field, Schema};
use parking_lot::Mutex;
use regex::Regex;
use sqlparser::ast::Ident;

use datafusion::execution::context::{FunctionFactory, RegisterFunction, SessionState};
use datafusion::prelude::*;
use datafusion::{execution::registry::FunctionRegistry, test_util};
Expand All @@ -48,6 +44,10 @@ use datafusion_expr::{
Volatility,
};
use datafusion_functions_nested::range::range_udf;
use parking_lot::Mutex;
use regex::Regex;
use sqlparser::ast::Ident;
use sqlparser::tokenizer::Span;

/// test that casting happens on udfs.
/// c11 is f32, but `custom_sqrt` requires f64. Casting happens but the logical plan and
Expand Down Expand Up @@ -1187,6 +1187,7 @@ async fn create_scalar_function_from_sql_statement_postgres_syntax() -> Result<(
name: Some(Ident {
value: "name".into(),
quote_style: None,
span: Span::empty(),
}),
data_type: DataType::Utf8,
default_expr: None,
Expand All @@ -1196,6 +1197,7 @@ async fn create_scalar_function_from_sql_statement_postgres_syntax() -> Result<(
language: Some(Ident {
value: "plrust".into(),
quote_style: None,
span: Span::empty(),
}),
behavior: None,
function_body: Some(lit(body)),
Expand Down
2 changes: 1 addition & 1 deletion datafusion/expr/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ pub enum Expr {
/// plan into physical plan.
Wildcard {
qualifier: Option<TableReference>,
options: WildcardOptions,
options: Box<WildcardOptions>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was needed to avoid a clippy lint about enum variants being too different. Specifically sqlparser added some new fields to WildCardOptions which made them substantially larger

},
/// List of grouping set expressions. Only valid in the context of an aggregate
/// GROUP BY expression list
Expand Down
8 changes: 4 additions & 4 deletions datafusion/expr/src/expr_fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,15 @@ pub fn placeholder(id: impl Into<String>) -> Expr {
pub fn wildcard() -> Expr {
Expr::Wildcard {
qualifier: None,
options: WildcardOptions::default(),
options: Box::new(WildcardOptions::default()),
}
}

/// Create an '*' [`Expr::Wildcard`] expression with the wildcard options
pub fn wildcard_with_options(options: WildcardOptions) -> Expr {
Expr::Wildcard {
qualifier: None,
options,
options: Box::new(options),
}
}

Expand All @@ -148,7 +148,7 @@ pub fn wildcard_with_options(options: WildcardOptions) -> Expr {
pub fn qualified_wildcard(qualifier: impl Into<TableReference>) -> Expr {
Expr::Wildcard {
qualifier: Some(qualifier.into()),
options: WildcardOptions::default(),
options: Box::new(WildcardOptions::default()),
}
}

Expand All @@ -159,7 +159,7 @@ pub fn qualified_wildcard_with_options(
) -> Expr {
Expr::Wildcard {
qualifier: Some(qualifier.into()),
options,
options: Box::new(options),
}
}

Expand Down
8 changes: 2 additions & 6 deletions datafusion/optimizer/src/analyzer/inline_table_scan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ use crate::analyzer::AnalyzerRule;
use datafusion_common::config::ConfigOptions;
use datafusion_common::tree_node::{Transformed, TransformedResult, TreeNode};
use datafusion_common::{Column, Result};
use datafusion_expr::expr::WildcardOptions;
use datafusion_expr::{logical_plan::LogicalPlan, Expr, LogicalPlanBuilder};
use datafusion_expr::{logical_plan::LogicalPlan, wildcard, Expr, LogicalPlanBuilder};

/// Analyzed rule that inlines TableScan that provide a [`LogicalPlan`]
/// (DataFrame / ViewTable)
Expand Down Expand Up @@ -93,10 +92,7 @@ fn generate_projection_expr(
)));
}
} else {
exprs.push(Expr::Wildcard {
qualifier: None,
options: WildcardOptions::default(),
});
exprs.push(wildcard());
}
Ok(exprs)
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/proto/src/logical_plan/from_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ pub fn parse_expr(
let qualifier = qualifier.to_owned().map(|x| x.try_into()).transpose()?;
Ok(Expr::Wildcard {
qualifier,
options: WildcardOptions::default(),
options: Box::new(WildcardOptions::default()),
})
}
ExprType::ScalarUdfExpr(protobuf::ScalarUdfExprNode {
Expand Down
12 changes: 3 additions & 9 deletions datafusion/proto/tests/cases/roundtrip_logical_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ use datafusion_common::{
use datafusion_expr::dml::CopyTo;
use datafusion_expr::expr::{
self, Between, BinaryExpr, Case, Cast, GroupingSet, InList, Like, ScalarFunction,
Unnest, WildcardOptions,
Unnest,
};
use datafusion_expr::logical_plan::{Extension, UserDefinedLogicalNodeCore};
use datafusion_expr::{
Expand Down Expand Up @@ -2061,21 +2061,15 @@ fn roundtrip_unnest() {

#[test]
fn roundtrip_wildcard() {
let test_expr = Expr::Wildcard {
qualifier: None,
options: WildcardOptions::default(),
};
let test_expr = wildcard();

let ctx = SessionContext::new();
roundtrip_expr_test(test_expr, ctx);
}

#[test]
fn roundtrip_qualified_wildcard() {
let test_expr = Expr::Wildcard {
qualifier: Some("foo".into()),
options: WildcardOptions::default(),
};
let test_expr = qualified_wildcard("foo");

let ctx = SessionContext::new();
roundtrip_expr_test(test_expr, ctx);
Expand Down
24 changes: 10 additions & 14 deletions datafusion/sql/src/expr/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ use datafusion_common::{
internal_datafusion_err, internal_err, not_impl_err, plan_datafusion_err, plan_err,
DFSchema, Dependency, Result,
};
use datafusion_expr::expr::WildcardOptions;
use datafusion_expr::expr::{ScalarFunction, Unnest};
use datafusion_expr::planner::PlannerResult;
use datafusion_expr::{
expr, Expr, ExprFunctionExt, ExprSchemable, WindowFrame, WindowFunctionDefinition,
expr, qualified_wildcard, wildcard, Expr, ExprFunctionExt, ExprSchemable,
WindowFrame, WindowFunctionDefinition,
};
use sqlparser::ast::{
DuplicateTreatment, Expr as SQLExpr, Function as SQLFunction, FunctionArg,
Expand Down Expand Up @@ -169,6 +169,11 @@ impl FunctionArgs {
"Calling {name}: SEPARATOR not supported in function arguments: {sep}"
)
}
FunctionArgumentClause::JsonNullClause(jn) => {
return not_impl_err!(
"Calling {name}: JSON NULL clause not supported in function arguments: {jn}"
)
}
}
}

Expand Down Expand Up @@ -413,28 +418,19 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
name: _,
arg: FunctionArgExpr::Wildcard,
operator: _,
} => Ok(Expr::Wildcard {
qualifier: None,
options: WildcardOptions::default(),
}),
} => Ok(wildcard()),
FunctionArg::Unnamed(FunctionArgExpr::Expr(arg)) => {
self.sql_expr_to_logical_expr(arg, schema, planner_context)
}
FunctionArg::Unnamed(FunctionArgExpr::Wildcard) => Ok(Expr::Wildcard {
qualifier: None,
options: WildcardOptions::default(),
}),
FunctionArg::Unnamed(FunctionArgExpr::Wildcard) => Ok(wildcard()),
FunctionArg::Unnamed(FunctionArgExpr::QualifiedWildcard(object_name)) => {
let qualifier = self.object_name_to_table_reference(object_name)?;
// Sanity check on qualifier with schema
let qualified_indices = schema.fields_indices_with_qualified(&qualifier);
if qualified_indices.is_empty() {
return plan_err!("Invalid qualifier {qualifier}");
}
Ok(Expr::Wildcard {
qualifier: Some(qualifier),
options: WildcardOptions::default(),
})
Ok(qualified_wildcard(qualifier))
}
_ => not_impl_err!("Unsupported qualified wildcard argument: {sql:?}"),
}
Expand Down
8 changes: 4 additions & 4 deletions datafusion/sql/src/expr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,13 +593,13 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
}
not_impl_err!("AnyOp not supported by ExprPlanner: {binary_expr:?}")
}
SQLExpr::Wildcard => Ok(Expr::Wildcard {
SQLExpr::Wildcard(_token) => Ok(Expr::Wildcard {
qualifier: None,
options: WildcardOptions::default(),
options: Box::new(WildcardOptions::default()),
}),
SQLExpr::QualifiedWildcard(object_name) => Ok(Expr::Wildcard {
SQLExpr::QualifiedWildcard(object_name, _token) => Ok(Expr::Wildcard {
qualifier: Some(self.object_name_to_table_reference(object_name)?),
options: WildcardOptions::default(),
options: Box::new(WildcardOptions::default()),
}),
SQLExpr::Tuple(values) => self.parse_tuple(schema, planner_context, values),
_ => not_impl_err!("Unsupported ast node in sqltorel: {sql:?}"),
Expand Down
14 changes: 12 additions & 2 deletions datafusion/sql/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ use std::collections::VecDeque;
use std::fmt;

use sqlparser::ast::ExprWithAlias;
use sqlparser::tokenizer::TokenWithSpan;
use sqlparser::{
ast::{
ColumnDef, ColumnOptionDef, ObjectName, OrderByExpr, Query,
Statement as SQLStatement, TableConstraint, Value,
},
dialect::{keywords::Keyword, Dialect, GenericDialect},
parser::{Parser, ParserError},
tokenizer::{Token, TokenWithLocation, Tokenizer, Word},
tokenizer::{Token, Tokenizer, Word},
};

// Use `Parser::expected` instead, if possible
Expand Down Expand Up @@ -338,7 +339,7 @@ impl<'a> DFParser<'a> {
fn expected<T>(
&self,
expected: &str,
found: TokenWithLocation,
found: TokenWithSpan,
) -> Result<T, ParserError> {
parser_err!(format!("Expected {expected}, found: {found}"))
}
Expand Down Expand Up @@ -876,6 +877,7 @@ mod tests {
use super::*;
use sqlparser::ast::Expr::Identifier;
use sqlparser::ast::{BinaryOperator, DataType, Expr, Ident};
use sqlparser::tokenizer::Span;

fn expect_parse_ok(sql: &str, expected: Statement) -> Result<(), ParserError> {
let statements = DFParser::parse_sql(sql)?;
Expand Down Expand Up @@ -911,6 +913,7 @@ mod tests {
name: Ident {
value: name.into(),
quote_style: None,
span: Span::empty(),
},
data_type,
collation: None,
Expand Down Expand Up @@ -1219,6 +1222,7 @@ mod tests {
expr: Identifier(Ident {
value: "c1".to_owned(),
quote_style: None,
span: Span::empty(),
}),
asc,
nulls_first,
Expand Down Expand Up @@ -1250,6 +1254,7 @@ mod tests {
expr: Identifier(Ident {
value: "c1".to_owned(),
quote_style: None,
span: Span::empty(),
}),
asc: Some(true),
nulls_first: None,
Expand All @@ -1259,6 +1264,7 @@ mod tests {
expr: Identifier(Ident {
value: "c2".to_owned(),
quote_style: None,
span: Span::empty(),
}),
asc: Some(false),
nulls_first: Some(true),
Expand Down Expand Up @@ -1290,11 +1296,13 @@ mod tests {
left: Box::new(Identifier(Ident {
value: "c1".to_owned(),
quote_style: None,
span: Span::empty(),
})),
op: BinaryOperator::Minus,
right: Box::new(Identifier(Ident {
value: "c2".to_owned(),
quote_style: None,
span: Span::empty(),
})),
},
asc: Some(true),
Expand Down Expand Up @@ -1335,11 +1343,13 @@ mod tests {
left: Box::new(Identifier(Ident {
value: "c1".to_owned(),
quote_style: None,
span: Span::empty(),
})),
op: BinaryOperator::Minus,
right: Box::new(Identifier(Ident {
value: "c2".to_owned(),
quote_style: None,
span: Span::empty(),
})),
},
asc: Some(true),
Expand Down
Loading
Loading