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

build(deps): upgrade sqlparser to 0.47.0 #10392

Merged
merged 41 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
7393c64
build(deps): upgrade sqlparser to 0.46.0
tisonkun May 5, 2024
d64ac06
function and cast fixups
jmhain May 6, 2024
c112222
catchup refactors
tisonkun May 8, 2024
f29af9d
try migrate json expr
tisonkun May 14, 2024
1ac63fa
Update for changes in sqlparser
alamb May 14, 2024
f973c0d
Merge remote-tracking branch 'apache/main' into upgrade-sqlparser
alamb May 14, 2024
d6da6fc
Update dependencies
alamb May 14, 2024
f553243
handle zero argument form
alamb May 14, 2024
f8eed01
fmt
alamb May 14, 2024
29fac1e
Merge branch 'main' into upgrade-sqlparser
tisonkun May 19, 2024
35d66e7
fixup more
tisonkun May 19, 2024
60ecc27
fixup more
tisonkun May 19, 2024
a150073
Merge branch 'main' into upgrade-sqlparser
tisonkun May 28, 2024
4fec8ee
try use jmhain's branch
tisonkun May 28, 2024
7e665e9
fix compile FunctionArgumentClause exhausted
tisonkun May 28, 2024
a5f8568
fix compile set multi vars
tisonkun May 28, 2024
545abae
fix compile new string values
tisonkun May 28, 2024
c8d5ea0
fix compile set multi vars
tisonkun May 28, 2024
598260f
fix compile Subscript
tisonkun May 28, 2024
e399416
cargo fmt
tisonkun May 28, 2024
a4aca5a
revert workaround on values
tisonkun May 28, 2024
042e7e8
Merge remote-tracking branch 'apache/main' into upgrade-sqlparser
alamb May 30, 2024
3cffded
Rework field access
alamb May 30, 2024
1bd355a
update lock
alamb May 30, 2024
98b539b
fix doc
alamb May 30, 2024
8efc69e
Merge remote-tracking branch 'apache/main' into upgrade-sqlparser
alamb May 30, 2024
53c72f5
try catchup new sqlparser version
tisonkun Jun 1, 2024
4d74ef6
fixup timezone expr
tisonkun Jun 1, 2024
c1163ad
fixup params
tisonkun Jun 1, 2024
e1d0c6b
lock
tisonkun Jun 1, 2024
685025a
Update to sqlparser 0.47.0
alamb Jun 1, 2024
ba24a7c
Merge remote-tracking branch 'apache/main' into upgrade-sqlparser
alamb Jun 1, 2024
b5743d5
Update rust stack size on windows
alamb Jun 1, 2024
d655ad6
Revert "Update rust stack size on windows"
alamb Jun 1, 2024
b8d84d5
Merge remote-tracking branch 'apache/main' into upgrade-sqlparser
alamb Jun 4, 2024
8446851
Add test + support for `$$` function definition
alamb Jun 4, 2024
72f61b0
Disable failing windows CI test
alamb Jun 4, 2024
089b571
fmt
alamb Jun 4, 2024
c3e475b
simplify test
alamb Jun 4, 2024
50a5a0f
fmt
alamb Jun 4, 2024
2290a8e
Merge remote-tracking branch 'apache/main' into upgrade-sqlparser
alamb Jun 6, 2024
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 @@ -111,7 +111,7 @@ rand = "0.8"
regex = "1.8"
rstest = "0.20.0"
serde_json = "1"
sqlparser = { version = "0.45.0", features = ["visitor"] }
sqlparser = { version = "0.47", 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.

2 changes: 1 addition & 1 deletion datafusion-examples/examples/function_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ impl TryFrom<CreateFunction> for ScalarFunctionWrapper {
name: definition.name,
expr: definition
.params
.return_
.function_body
.expect("Expression has to be defined!"),
return_type: definition
.return_type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ impl TryFrom<CreateFunction> for ScalarFunctionWrapper {
name: definition.name,
expr: definition
.params
.return_
.function_body
.expect("Expression has to be defined!"),
return_type: definition
.return_type
Expand Down
25 changes: 2 additions & 23 deletions datafusion/expr/src/logical_plan/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,29 +341,8 @@ pub struct CreateFunctionBody {
pub language: Option<Ident>,
/// IMMUTABLE | STABLE | VOLATILE
pub behavior: Option<Volatility>,
/// AS 'definition'
pub as_: Option<DefinitionStatement>,
/// RETURN expression
pub return_: Option<Expr>,
}

#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub enum DefinitionStatement {
SingleQuotedDef(String),
DoubleDollarDef(String),
}

impl From<sqlparser::ast::FunctionDefinition> for DefinitionStatement {
fn from(value: sqlparser::ast::FunctionDefinition) -> Self {
match value {
sqlparser::ast::FunctionDefinition::SingleQuotedDef(s) => {
Self::SingleQuotedDef(s)
}
sqlparser::ast::FunctionDefinition::DoubleDollarDef(s) => {
Self::DoubleDollarDef(s)
}
}
}
/// RETURN or AS function body
pub function_body: Option<Expr>,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI @milenkovicm -- sqlparser changed how the body was represented so now this changes as well to a generic Expr

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for letting me know, @alamb

I just wonder if this change will diverge sqlparser from Postgres?

I guess function like this

CREATE FUNCTION strlen(name TEXT)
    RETURNS int LANGUAGE plrust AS
$$
    Ok(Some(name.unwrap().len() as i32))
$$;

Would not be possible without wrapping body with string

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like we may need to add $$ string parsing support in sqlparser. I'll try and file a ticket later todday

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC we should previously support $$ function body. If it fails to parse now, it's a regression.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I added a test and made this syntax work in this pr in 8446851

}

#[derive(Clone, PartialEq, Eq, Hash, Debug)]
Expand Down
4 changes: 2 additions & 2 deletions datafusion/expr/src/logical_plan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ pub use builder::{
};
pub use ddl::{
CreateCatalog, CreateCatalogSchema, CreateExternalTable, CreateFunction,
CreateFunctionBody, CreateMemoryTable, CreateView, DdlStatement, DefinitionStatement,
DropCatalogSchema, DropFunction, DropTable, DropView, OperateFunctionArg,
CreateFunctionBody, CreateMemoryTable, CreateView, DdlStatement, DropCatalogSchema,
DropFunction, DropTable, DropView, OperateFunctionArg,
};
pub use dml::{DmlStatement, WriteOp};
pub use plan::{
Expand Down
2 changes: 2 additions & 0 deletions datafusion/sql/src/expr/binary_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
BinaryOperator::PGBitwiseShiftRight => Ok(Operator::BitwiseShiftRight),
BinaryOperator::PGBitwiseShiftLeft => Ok(Operator::BitwiseShiftLeft),
BinaryOperator::StringConcat => Ok(Operator::StringConcat),
BinaryOperator::ArrowAt => Ok(Operator::ArrowAt),
BinaryOperator::AtArrow => Ok(Operator::AtArrow),
_ => not_impl_err!("Unsupported SQL binary operator {op:?}"),
}
}
Expand Down
128 changes: 122 additions & 6 deletions datafusion/sql/src/expr/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ use datafusion_expr::{
BuiltInWindowFunction,
};
use sqlparser::ast::{
Expr as SQLExpr, Function as SQLFunction, FunctionArg, FunctionArgExpr, WindowType,
DuplicateTreatment, Expr as SQLExpr, Function as SQLFunction, FunctionArg,
FunctionArgExpr, FunctionArgumentClause, FunctionArgumentList, FunctionArguments,
NullTreatment, ObjectName, OrderByExpr, WindowType,
};
use std::str::FromStr;
use strum::IntoEnumIterator;
Expand Down Expand Up @@ -79,23 +81,137 @@ fn find_closest_match(candidates: Vec<String>, target: &str) -> String {
.expect("No candidates provided.") // Panic if `candidates` argument is empty
}

/// Arguments to for a function call extracted from the SQL AST
#[derive(Debug)]
struct FunctionArgs {
/// Function name
name: ObjectName,
/// Argument expressions
args: Vec<FunctionArg>,
/// ORDER BY clause, if any
order_by: Vec<OrderByExpr>,
/// OVER clause, if any
over: Option<WindowType>,
/// FILTER clause, if any
filter: Option<Box<SQLExpr>>,
/// NULL treatment clause, if any
null_treatment: Option<NullTreatment>,
/// DISTINCT
distinct: bool,
}

impl FunctionArgs {
fn try_new(function: SQLFunction) -> Result<Self> {
let SQLFunction {
name,
args,
over,
filter,
mut null_treatment,
within_group,
} = function;

// Handle no argument form (aka `current_time` as opposed to `current_time()`)
let FunctionArguments::List(args) = args else {
return Ok(Self {
name,
args: vec![],
order_by: vec![],
over,
filter,
null_treatment,
distinct: false,
});
};

let FunctionArgumentList {
tisonkun marked this conversation as resolved.
Show resolved Hide resolved
duplicate_treatment,
args,
clauses,
} = args;

let distinct = match duplicate_treatment {
Some(DuplicateTreatment::Distinct) => true,
Some(DuplicateTreatment::All) => false,
None => false,
};

// Pull out argument handling
let mut order_by = None;
for clause in clauses {
match clause {
FunctionArgumentClause::IgnoreOrRespectNulls(nt) => {
if null_treatment.is_some() {
return not_impl_err!(
"Calling {name}: Duplicated null treatment clause"
);
}
null_treatment = Some(nt);
}
FunctionArgumentClause::OrderBy(oby) => {
if order_by.is_some() {
return not_impl_err!("Calling {name}: Duplicated ORDER BY clause in function arguments");
}
order_by = Some(oby);
}
FunctionArgumentClause::Limit(limit) => {
return not_impl_err!(
"Calling {name}: LIMIT not supported in function arguments: {limit}"
)
}
FunctionArgumentClause::OnOverflow(overflow) => {
return not_impl_err!(
"Calling {name}: ON OVERFLOW not supported in function arguments: {overflow}"
)
}
FunctionArgumentClause::Having(having) => {
return not_impl_err!(
"Calling {name}: HAVING not supported in function arguments: {having}"
)
}
FunctionArgumentClause::Separator(sep) => {
return not_impl_err!(
"Calling {name}: SEPARATOR not supported in function arguments: {sep}"
)
}
}
}

if !within_group.is_empty() {
return not_impl_err!("WITHIN GROUP is not supported yet: {within_group:?}");
}

let order_by = order_by.unwrap_or_default();

Ok(Self {
name,
args,
order_by,
over,
filter,
null_treatment,
distinct,
})
}
}

impl<'a, S: ContextProvider> SqlToRel<'a, S> {
pub(super) fn sql_function_to_expr(
&self,
function: SQLFunction,
schema: &DFSchema,
planner_context: &mut PlannerContext,
) -> Result<Expr> {
let SQLFunction {
let function_args = FunctionArgs::try_new(function)?;
let FunctionArgs {
name,
args,
order_by,
over,
distinct,
filter,
null_treatment,
special: _, // true if not called with trailing parens
order_by,
} = function;
distinct,
} = function_args;

// If function is a window function (it has an OVER clause),
// it shouldn't have ordering requirement as function argument
Expand Down
31 changes: 0 additions & 31 deletions datafusion/sql/src/expr/json_access.rs

This file was deleted.

Loading
Loading