Skip to content

Commit

Permalink
upgrade sqlparser 0.47 -> 0.48 (#11453)
Browse files Browse the repository at this point in the history
* upgrade sqlparser 0.47 -> 0.48

* clean imports and qualified imports

* update df-cli cargo lock

* fix trailing commas in slt tests

* update slt tests results

* restore rowsort in slt tests

* fix slt tests

* rerun CI

* reset unchanged slt files

* Revert "clean imports and qualified imports"

This reverts commit 7be2263.

* update non-windows systems stack size

* update windows stack size

* remove windows-only unused import

* use same test main for all systems

* Reapply "clean imports and qualified imports"

This reverts commit 4fc036a.
  • Loading branch information
MohamedAbdeen21 committed Jul 16, 2024
1 parent c54a638 commit 382bf4f
Show file tree
Hide file tree
Showing 32 changed files with 123 additions and 105 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ rand = "0.8"
regex = "1.8"
rstest = "0.21.0"
serde_json = "1"
sqlparser = { version = "0.47", features = ["visitor"] }
sqlparser = { version = "0.48", features = ["visitor"] }
tempfile = "3"
thiserror = "1.0.44"
tokio = { version = "1.36", features = ["macros", "rt", "sync"] }
Expand Down
4 changes: 2 additions & 2 deletions datafusion-cli/Cargo.lock

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

1 change: 1 addition & 0 deletions datafusion/sql/src/expr/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ impl FunctionArgs {
filter,
mut null_treatment,
within_group,
..
} = function;

// Handle no argument form (aka `current_time` as opposed to `current_time()`)
Expand Down
18 changes: 11 additions & 7 deletions datafusion/sql/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1006,14 +1006,15 @@ mod tests {
expect_parse_ok(sql, expected)?;

// positive case: it is ok for sql stmt with `COMPRESSION TYPE GZIP` tokens
let sqls = vec![
("CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV LOCATION 'foo.csv' OPTIONS
let sqls =
vec![
("CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV LOCATION 'foo.csv' OPTIONS
('format.compression' 'GZIP')", "GZIP"),
("CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV LOCATION 'foo.csv' OPTIONS
("CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV LOCATION 'foo.csv' OPTIONS
('format.compression' 'BZIP2')", "BZIP2"),
("CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV LOCATION 'foo.csv' OPTIONS
("CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV LOCATION 'foo.csv' OPTIONS
('format.compression' 'XZ')", "XZ"),
("CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV LOCATION 'foo.csv' OPTIONS
("CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV LOCATION 'foo.csv' OPTIONS
('format.compression' 'ZSTD')", "ZSTD"),
];
for (sql, compression) in sqls {
Expand Down Expand Up @@ -1123,7 +1124,10 @@ mod tests {
// negative case: mixed column defs and column names in `PARTITIONED BY` clause
let sql =
"CREATE EXTERNAL TABLE t(c1 int) STORED AS CSV PARTITIONED BY (p1 int, c1) LOCATION 'foo.csv'";
expect_parse_error(sql, "sql parser error: Expected a data type name, found: )");
expect_parse_error(
sql,
"sql parser error: Expected: a data type name, found: )",
);

// negative case: mixed column defs and column names in `PARTITIONED BY` clause
let sql =
Expand Down Expand Up @@ -1291,7 +1295,7 @@ mod tests {
LOCATION 'foo.parquet'
OPTIONS ('format.compression' 'zstd',
'format.delimiter' '*',
'ROW_GROUP_SIZE' '1024',
'ROW_GROUP_SIZE' '1024',
'TRUNCATE' 'NO',
'format.has_header' 'true')";
let expected = Statement::CreateExternalTable(CreateExternalTable {
Expand Down
21 changes: 21 additions & 0 deletions datafusion/sql/src/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,27 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
| SQLDataType::Float64
| SQLDataType::JSONB
| SQLDataType::Unspecified
// Clickhouse datatypes
| SQLDataType::Int16
| SQLDataType::Int32
| SQLDataType::Int128
| SQLDataType::Int256
| SQLDataType::UInt8
| SQLDataType::UInt16
| SQLDataType::UInt32
| SQLDataType::UInt64
| SQLDataType::UInt128
| SQLDataType::UInt256
| SQLDataType::Float32
| SQLDataType::Date32
| SQLDataType::Datetime64(_, _)
| SQLDataType::FixedString(_)
| SQLDataType::Map(_, _)
| SQLDataType::Tuple(_)
| SQLDataType::Nested(_)
| SQLDataType::Union(_)
| SQLDataType::Nullable(_)
| SQLDataType::LowCardinality(_)
=> not_impl_err!(
"Unsupported SQL type {sql_type:?}"
),
Expand Down
2 changes: 1 addition & 1 deletion datafusion/sql/src/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
let aggr_exprs = find_aggregate_exprs(&aggr_expr_haystack);

// All of the group by expressions
let group_by_exprs = if let GroupByExpr::Expressions(exprs) = select.group_by {
let group_by_exprs = if let GroupByExpr::Expressions(exprs, _) = select.group_by {
exprs
.into_iter()
.map(|e| {
Expand Down
60 changes: 31 additions & 29 deletions datafusion/sql/src/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,19 @@ use datafusion_expr::{
cast, col, Analyze, CreateCatalog, CreateCatalogSchema,
CreateExternalTable as PlanCreateExternalTable, CreateFunction, CreateFunctionBody,
CreateMemoryTable, CreateView, DescribeTable, DmlStatement, DropCatalogSchema,
DropFunction, DropTable, DropView, EmptyRelation, Explain, ExprSchemable, Filter,
LogicalPlan, LogicalPlanBuilder, OperateFunctionArg, PlanType, Prepare, SetVariable,
Statement as PlanStatement, ToStringifiedPlan, TransactionAccessMode,
DropFunction, DropTable, DropView, EmptyRelation, Explain, Expr, ExprSchemable,
Filter, LogicalPlan, LogicalPlanBuilder, OperateFunctionArg, PlanType, Prepare,
SetVariable, Statement as PlanStatement, ToStringifiedPlan, TransactionAccessMode,
TransactionConclusion, TransactionEnd, TransactionIsolationLevel, TransactionStart,
Volatility, WriteOp,
};
use sqlparser::ast;
use sqlparser::ast::{
Assignment, ColumnDef, CreateTableOptions, Delete, DescribeAlias, Expr as SQLExpr,
Expr, FromTable, Ident, Insert, ObjectName, ObjectType, OneOrManyWithParens, Query,
SchemaName, SetExpr, ShowCreateObject, ShowStatementFilter, Statement,
TableConstraint, TableFactor, TableWithJoins, TransactionMode, UnaryOperator, Value,
Assignment, AssignmentTarget, ColumnDef, CreateTable, CreateTableOptions, Delete,
DescribeAlias, Expr as SQLExpr, FromTable, Ident, Insert, ObjectName, ObjectType,
OneOrManyWithParens, Query, SchemaName, SetExpr, ShowCreateObject,
ShowStatementFilter, Statement, TableConstraint, TableFactor, TableWithJoins,
TransactionMode, UnaryOperator, Value,
};
use sqlparser::parser::ParserError::ParserError;

Expand Down Expand Up @@ -240,7 +241,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
value,
} => self.set_variable_to_plan(local, hivevar, &variables, value),

Statement::CreateTable {
Statement::CreateTable(CreateTable {
query,
name,
columns,
Expand All @@ -250,7 +251,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
if_not_exists,
or_replace,
..
} if table_properties.is_empty() && with_options.is_empty() => {
}) if table_properties.is_empty() && with_options.is_empty() => {
// Merge inline constraints and existing constraints
let mut all_constraints = constraints;
let inline_constraints = calc_inline_constraints_from_columns(&columns);
Expand Down Expand Up @@ -954,7 +955,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
order_exprs: Vec<LexOrdering>,
schema: &DFSchemaRef,
planner_context: &mut PlannerContext,
) -> Result<Vec<Vec<datafusion_expr::Expr>>> {
) -> Result<Vec<Vec<Expr>>> {
// Ask user to provide a schema if schema is empty.
if !order_exprs.is_empty() && schema.fields().is_empty() {
return plan_err!(
Expand Down Expand Up @@ -1159,7 +1160,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
local: bool,
hivevar: bool,
variables: &OneOrManyWithParens<ObjectName>,
value: Vec<Expr>,
value: Vec<SQLExpr>,
) -> Result<LogicalPlan> {
if local {
return not_impl_err!("LOCAL is not supported");
Expand Down Expand Up @@ -1218,7 +1219,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
fn delete_to_plan(
&self,
table_name: ObjectName,
predicate_expr: Option<Expr>,
predicate_expr: Option<SQLExpr>,
) -> Result<LogicalPlan> {
// Do a table lookup to verify the table exists
let table_ref = self.object_name_to_table_reference(table_name.clone())?;
Expand Down Expand Up @@ -1264,7 +1265,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
table: TableWithJoins,
assignments: Vec<Assignment>,
from: Option<TableWithJoins>,
predicate_expr: Option<Expr>,
predicate_expr: Option<SQLExpr>,
) -> Result<LogicalPlan> {
let (table_name, table_alias) = match &table.relation {
TableFactor::Table { name, alias, .. } => (name.clone(), alias.clone()),
Expand All @@ -1284,16 +1285,20 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
let mut assign_map = assignments
.iter()
.map(|assign| {
let col_name: &Ident = assign
.id
let cols = match &assign.target {
AssignmentTarget::ColumnName(cols) => cols,
_ => plan_err!("Tuples are not supported")?,
};
let col_name: &Ident = cols
.0
.iter()
.last()
.ok_or_else(|| plan_datafusion_err!("Empty column id"))?;
// Validate that the assignment target column exists
table_schema.field_with_unqualified_name(&col_name.value)?;
Ok((col_name.value.clone(), assign.value.clone()))
})
.collect::<Result<HashMap<String, Expr>>>()?;
.collect::<Result<HashMap<String, SQLExpr>>>()?;

// Build scan, join with from table if it exists.
let mut input_tables = vec![table];
Expand Down Expand Up @@ -1332,8 +1337,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
&mut planner_context,
)?;
// Update placeholder's datatype to the type of the target column
if let datafusion_expr::Expr::Placeholder(placeholder) = &mut expr
{
if let Expr::Placeholder(placeholder) = &mut expr {
placeholder.data_type = placeholder
.data_type
.take()
Expand All @@ -1345,14 +1349,12 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
None => {
// If the target table has an alias, use it to qualify the column name
if let Some(alias) = &table_alias {
datafusion_expr::Expr::Column(Column::new(
Expr::Column(Column::new(
Some(self.normalizer.normalize(alias.name.clone())),
field.name(),
))
} else {
datafusion_expr::Expr::Column(Column::from((
qualifier, field,
)))
Expr::Column(Column::from((qualifier, field)))
}
}
};
Expand Down Expand Up @@ -1427,7 +1429,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
if let SetExpr::Values(ast::Values { rows, .. }) = (*source.body).clone() {
for row in rows.iter() {
for (idx, val) in row.iter().enumerate() {
if let ast::Expr::Value(Value::Placeholder(name)) = val {
if let SQLExpr::Value(Value::Placeholder(name)) = val {
let name =
name.replace('$', "").parse::<usize>().map_err(|_| {
plan_datafusion_err!("Can't parse placeholder: {name}")
Expand Down Expand Up @@ -1460,23 +1462,23 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
.map(|(i, value_index)| {
let target_field = table_schema.field(i);
let expr = match value_index {
Some(v) => datafusion_expr::Expr::Column(Column::from(
source.schema().qualified_field(v),
))
.cast_to(target_field.data_type(), source.schema())?,
Some(v) => {
Expr::Column(Column::from(source.schema().qualified_field(v)))
.cast_to(target_field.data_type(), source.schema())?
}
// The value is not specified. Fill in the default value for the column.
None => table_source
.get_column_default(target_field.name())
.cloned()
.unwrap_or_else(|| {
// If there is no default for the column, then the default is NULL
datafusion_expr::Expr::Literal(ScalarValue::Null)
Expr::Literal(ScalarValue::Null)
})
.cast_to(target_field.data_type(), &DFSchema::empty())?,
};
Ok(expr.alias(target_field.name()))
})
.collect::<Result<Vec<datafusion_expr::Expr>>>()?;
.collect::<Result<Vec<Expr>>>()?;
let source = project(source, exprs)?;

let op = if overwrite {
Expand Down
5 changes: 4 additions & 1 deletion datafusion/sql/src/unparser/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ impl QueryBuilder {
fetch: self.fetch.clone(),
locks: self.locks.clone(),
for_clause: self.for_clause.clone(),
settings: None,
format_clause: None,
})
}
fn create_empty() -> Self {
Expand Down Expand Up @@ -234,6 +236,7 @@ impl SelectBuilder {
value_table_mode: self.value_table_mode,
connect_by: None,
window_before_qualify: false,
prewhere: None,
})
}
fn create_empty() -> Self {
Expand All @@ -245,7 +248,7 @@ impl SelectBuilder {
from: Default::default(),
lateral_views: Default::default(),
selection: Default::default(),
group_by: Some(ast::GroupByExpr::Expressions(Vec::new())),
group_by: Some(ast::GroupByExpr::Expressions(Vec::new(), Vec::new())),
cluster_by: Default::default(),
distribute_by: Default::default(),
sort_by: Default::default(),
Expand Down
3 changes: 3 additions & 0 deletions datafusion/sql/src/unparser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ impl Unparser<'_> {
null_treatment: None,
over: None,
within_group: vec![],
parameters: ast::FunctionArguments::None,
}))
}
Expr::Between(Between {
Expand Down Expand Up @@ -306,6 +307,7 @@ impl Unparser<'_> {
null_treatment: None,
over,
within_group: vec![],
parameters: ast::FunctionArguments::None,
}))
}
Expr::SimilarTo(Like {
Expand Down Expand Up @@ -351,6 +353,7 @@ impl Unparser<'_> {
null_treatment: None,
over: None,
within_group: vec![],
parameters: ast::FunctionArguments::None,
}))
}
Expr::ScalarSubquery(subq) => {
Expand Down
1 change: 1 addition & 0 deletions datafusion/sql/src/unparser/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ impl Unparser<'_> {
.iter()
.map(|expr| self.expr_to_sql(expr))
.collect::<Result<Vec<_>>>()?,
vec![],
));
}
Some(AggVariant::Window(window)) => {
Expand Down
6 changes: 3 additions & 3 deletions datafusion/sql/tests/sql_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3627,7 +3627,7 @@ fn test_prepare_statement_to_plan_panic_prepare_wrong_syntax() {
let sql = "PREPARE AS SELECT id, age FROM person WHERE age = $foo";
assert_eq!(
logical_plan(sql).unwrap_err().strip_backtrace(),
"SQL error: ParserError(\"Expected AS, found: SELECT\")"
"SQL error: ParserError(\"Expected: AS, found: SELECT\")"
)
}

Expand Down Expand Up @@ -3668,7 +3668,7 @@ fn test_non_prepare_statement_should_infer_types() {

#[test]
#[should_panic(
expected = "value: SQL(ParserError(\"Expected [NOT] NULL or TRUE|FALSE or [NOT] DISTINCT FROM after IS, found: $1\""
expected = "value: SQL(ParserError(\"Expected: [NOT] NULL or TRUE|FALSE or [NOT] DISTINCT FROM after IS, found: $1\""
)]
fn test_prepare_statement_to_plan_panic_is_param() {
let sql = "PREPARE my_plan(INT) AS SELECT id, age FROM person WHERE age is $1";
Expand Down Expand Up @@ -4347,7 +4347,7 @@ fn test_parse_escaped_string_literal_value() {
let sql = r"SELECT character_length(E'\000') AS len";
assert_eq!(
logical_plan(sql).unwrap_err().strip_backtrace(),
"SQL error: TokenizerError(\"Unterminated encoded string literal at Line: 1, Column 25\")"
"SQL error: TokenizerError(\"Unterminated encoded string literal at Line: 1, Column: 25\")"
)
}

Expand Down
Loading

0 comments on commit 382bf4f

Please sign in to comment.