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

feat(tql): add initial support for start,stop,step as sql functions #3507

Merged

Conversation

etolbakov
Copy link
Collaborator

@etolbakov etolbakov commented Mar 13, 2024

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

#3452
#3453 (lookback parameter)

What's changed and what's your intention?

TQL parser accepts SQL functions such as now() for start,end and step parameters:

Please explain IN DETAIL what the changes are in this PR and why they are needed:

  • enhanced the tql_parser logic with small pre-planning and pre-executing phases to evaluate start,end and step into number (timestamp) if those parameter were passed as SQL functions; The logic is the following
    1. converting the function into “logical” expr
    2. using simplifier evaluate expr.

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR does not require documentation updates.

@github-actions github-actions bot added the docs-not-required This change does not impact docs. label Mar 13, 2024
@waynexia waynexia self-requested a review March 14, 2024 13:25
@etolbakov
Copy link
Collaborator Author

etolbakov commented Mar 18, 2024

add lookback param support for TQL EVAL
#3453
Happy to add more logic to accomplish the ticket

@etolbakov etolbakov marked this pull request as ready for review March 18, 2024 16:14
Copy link

codecov bot commented Mar 18, 2024

Codecov Report

Attention: Patch coverage is 88.91403% with 49 lines in your changes are missing coverage. Please review.

Project coverage is 85.07%. Comparing base (63681f0) to head (1abf153).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3507      +/-   ##
==========================================
- Coverage   85.41%   85.07%   -0.35%     
==========================================
  Files         931      931              
  Lines      154531   154806     +275     
==========================================
- Hits       131997   131696     -301     
- Misses      22534    23110     +576     

Copy link
Member

@waynexia waynexia left a comment

Choose a reason for hiding this comment

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

Thanks for taking this! I left some style sugg.:

src/sql/src/parsers/tql_parser.rs Outdated Show resolved Hide resolved
src/sql/src/parsers/tql_parser.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@killme2008 killme2008 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!

src/sql/src/parsers/error.rs Show resolved Hide resolved
src/sql/src/statements/tql.rs Show resolved Hide resolved
src/sql/src/parsers/tql_parser.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@killme2008 killme2008 left a comment

Choose a reason for hiding this comment

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

LGTM

src/sql/src/parsers/tql_parser.rs Outdated Show resolved Hide resolved
Copy link
Member

@waynexia waynexia left a comment

Choose a reason for hiding this comment

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

Thanks for this!

@waynexia waynexia enabled auto-merge March 29, 2024 06:29
@waynexia waynexia added this pull request to the merge queue Mar 29, 2024
@etolbakov
Copy link
Collaborator Author

Thank you @waynexia for useful tips(especially with the simplification) and help during the implementation!
It was a great pleasure to collaborate on this ticket!

@waynexia
Copy link
Member

Thanks for keeping working on this huge and exciting feature. I can't wait to try it out!

Merged via the queue into GreptimeTeam:main with commit 14267c2 Mar 29, 2024
19 checks passed
@etolbakov etolbakov deleted the tql-start-stop-step-sql-functions-support branch March 29, 2024 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-not-required This change does not impact docs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants