Skip to content

Conversation

@aprimadi
Copy link
Contributor

@aprimadi aprimadi commented May 7, 2023

Which issue does this PR close?

Closes #6205.

Rationale for this change

Due to sqlparser lack of semantic, we need to rewrite the AST in datafusion.

For example, this is valid in sqlparser:

SELECT INTERVAL `1 month` + `1 month`

This is also valid:

SELECT INTERVAL '1 month' + '2012-01-01'::date;

Both generates structurally similar interval expression with BinaryOp +.

The first statement generates the following AST (SELECT omitted):

Interval
  BinaryOp
     Value(StringLiteral)
     Value(StringLiteral)

The second statement generates the following AST (SELECT omitted):

Interval
  BinaryOp
     Value(StringLiteral)
     Cast
       Value(StringLiteral)

I don't think this can be handled in sqlparser because sqlparser doesn't have the semantic to understand which BinaryOp + cast is and isn't allowed.

This PR transforms the 2nd AST (or any AST with non string literal value in right operand) into:

BinaryOp
  Interval
    Value(StringLiteral)
  Cast
     Value(StringLiteral)

What changes are included in this PR?

  • Modify parser to allow for interval + date in a select statement.
  • Add some test cases

Are these changes tested?

Yes

Are there any user-facing changes?

No

@github-actions github-actions bot added core Core DataFusion crate sql SQL Planner sqllogictest SQL Logic Tests (.slt) labels May 7, 2023
@aprimadi aprimadi marked this pull request as ready for review May 7, 2023 12:19
@aprimadi
Copy link
Contributor Author

aprimadi commented May 7, 2023

@jackwener I have more test cases coming up

@alamb alamb changed the title Handle binary op interval in logical AST builder Support interval '1 month' + date/timestamp: Handle binary op interval in logical AST builder May 8, 2023
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks great to me -- thank you @aprimadi

It is quite a series of PRs you have contributed recently 🏅

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks great to me -- thank you @aprimadi

It is quite a series of PRs you have contributed recently 🏅

aprimadi and others added 3 commits May 9, 2023 11:01
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks again @aprimadi

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks again @aprimadi

@alamb alamb merged commit 17e6526 into apache:main May 9, 2023
@aprimadi aprimadi deleted the ap-interval-date branch May 9, 2023 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

select interval '1 month' + date/timestamp doesn't work

3 participants