GreptimeTeam / greptimedb

An Open-Source, Cloud-Native, Unified Time Series Database for Metrics, Events, and Logs with SQL/PromQL supported. Available on GreptimeCloud.
https://greptime.com/
Apache License 2.0
4k stars 289 forks source link

Improve interval expression #4168

Closed killme2008 closed 2 weeks ago

killme2008 commented 4 weeks ago

What type of enhancement is this?

API improvement, User experience

What does the enhancement do?

GreptimeDB supports sql-compatible intervals like:

 select interval '1 hour';

That is great. But in many time-series query scenarios, the 1 hour is shortened to 1h. It is great to support it too:

mysql> select interval '1h';
ERROR 1815 (HY000): Failed to plan SQL: Not yet implemented: Unsupported Interval Expression with value "1h seconds"

Other units such as:

etc.

Implementation challenges

No response

etolbakov commented 3 weeks ago

Hi Dennis @killme2008 Do I get it right that the exception comes from the DataFusion?

If that's the case is there any established way of "intercepting" the AST before sending it to plan? Thank you!

killme2008 commented 3 weeks ago

Hi Dennis @killme2008 Do I get it right that the exception comes from the DataFusion?

If that's the case is there any established way of "intercepting" the AST before sending it to plan? Thank you!

I think we can transform the Expr just like type alias:

https://github.com/GreptimeTeam/greptimedb/blob/main/src/sql/src/statements/transform/type_alias.rs

Adds a new transform rule and add it to

https://github.com/GreptimeTeam/greptimedb/blob/cc2f7efb983fe057b4efa0cf625d7658a7b5e774/src/sql/src/statements/transform.rs#L29

killme2008 commented 3 weeks ago

@etolbakov Looks like compound signed number case not processed properly:

mysql> SELECT INTERVAL '-1h5m';
+----------------------------------------------+
| IntervalMonthDayNano("18446740773709551616") |
+----------------------------------------------+
| P0Y0M0DT0H-55M0S                             |
+----------------------------------------------+

mysql> SELECT INTERVAL '-1h-5m';
ERROR 1815 (HY000): Failed to plan SQL: Parser error: Invalid input syntax for type interval: "-1 h- 5 minutes"

Would you like to fix it?

etolbakov commented 3 weeks ago

ohhh....my bad, yeah, let me give it a go

killme2008 commented 3 weeks ago

I have crafted a document detailing the interval type. Would you kindly provide feedback upon review?

https://github.com/GreptimeTeam/docs/pull/1021

killme2008 commented 3 weeks ago

@etolbakov I have an idea. Maybe we can apply this transform rule to the cast(string as interval) expression too? Then it can be applied to the interval in the form of '3y2mon'::interval. What do you think about it?

https://docs.rs/datafusion/latest/datafusion/logical_expr/enum.Expr.html#variant.Cast

https://docs.rs/datafusion/latest/datafusion/logical_expr/struct.TryCast.html

etolbakov commented 3 weeks ago

It's a great idea! Let me make an assessment (and reopen the ticket 😁)