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
Add parser support for timestampadd
#857
Conversation
timestampadd
Token::Word(w) if w.value.to_lowercase() == "timestampadd" => { | ||
// TIMESTAMPADD(YEAR, 2, d) | ||
parser.next_token(); // skip timestampadd | ||
parser.expect_token(&Token::LParen)?; | ||
let time_unit = parser.next_token(); | ||
parser.expect_token(&Token::Comma)?; | ||
let n = parser.parse_expr()?; | ||
parser.expect_token(&Token::Comma)?; | ||
let expr = parser.parse_expr()?; | ||
parser.expect_token(&Token::RParen)?; | ||
|
||
// convert to function args | ||
let args = vec![ | ||
FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Value( | ||
Value::SingleQuotedString(time_unit.to_string()), | ||
))), | ||
FunctionArg::Unnamed(FunctionArgExpr::Expr(n)), | ||
FunctionArg::Unnamed(FunctionArgExpr::Expr(expr)), | ||
]; | ||
|
||
Ok(Some(Expr::Function(Function { | ||
name: ObjectName(vec![Ident::new("TIMESTAMPADD")]), | ||
args, | ||
over: None, | ||
distinct: false, | ||
special: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so cool! Definitely more convenient than having to go through SQLParser and DataFusion, I can already think of a couple more functions that I'll do this for.
]; | ||
|
||
Ok(Some(Expr::Function(Function { | ||
name: ObjectName(vec![Ident::new("TIMESTAMPADD")]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sake of consistency with other custom functions, could we give this an all lowercase name?
name: ObjectName(vec![Ident::new("TIMESTAMPADD")]), | |
name: ObjectName(vec![Ident::new("timestampadd")]), |
Codecov Report
@@ Coverage Diff @@
## main #857 +/- ##
==========================================
+ Coverage 77.44% 77.58% +0.13%
==========================================
Files 71 71
Lines 3600 3600
Branches 634 634
==========================================
+ Hits 2788 2793 +5
+ Misses 685 676 -9
- Partials 127 131 +4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Closing this now that these changes are part of #838 |
DataFusion's parser cannot parse
timestampadd(year, 2, d)
becauseyear
is not a keyword, so it assumes that this is an identifier and looks for a column with this name.This PR works around the issue by providing a custom prefix parser in the
DaskSqlDialect
where we can override parsing of just this expression and then fall back to DataFusion. This is a new feature that was added to sqlparser relatively recently.