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: Implement SHOW CREATE FLOW #4040

Merged
merged 14 commits into from
Jun 7, 2024
Merged

Conversation

irenjj
Copy link
Collaborator

@irenjj irenjj commented May 25, 2024

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

close: #3939

What's changed and what's your intention?

Implement SHOW CREATE FLOW

Checklist

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

@irenjj irenjj requested review from MichaelScofield, evenyag and a team as code owners May 25, 2024 03:08
@github-actions github-actions bot added the docs-not-required This change does not impact docs. label May 25, 2024
Copy link

codecov bot commented May 25, 2024

Codecov Report

Attention: Patch coverage is 41.80791% with 103 lines in your changes missing coverage. Please review.

Project coverage is 85.11%. Comparing base (03cacf9) to head (f848d71).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4040      +/-   ##
==========================================
- Coverage   85.48%   85.11%   -0.38%     
==========================================
  Files         993      994       +1     
  Lines      173751   173991     +240     
==========================================
- Hits       148539   148094     -445     
- Misses      25212    25897     +685     

@WenyXu
Copy link
Member

WenyXu commented May 25, 2024

Would you like to add some sqlness tests for SHOW FLOW?
BTW, It seems we need to add sqlness tests for creating flow and dropping flow cc @discord9 @waynexia

@irenjj
Copy link
Collaborator Author

irenjj commented May 25, 2024

Would you like to add some sqlness tests for SHOW FLOW? BTW, It seems we need to add sqlness tests for creating flow and dropping flow cc @discord9 @waynexia

Sure, i'll add tests after some review feedback, because the output format of SHOW CREATE FLOW might change.

src/query/src/sql.rs Outdated Show resolved Hide resolved
@discord9 discord9 mentioned this pull request May 27, 2024
8 tasks
@irenjj irenjj requested a review from evenyag June 1, 2024 03:57
src/operator/src/statement.rs Outdated Show resolved Hide resolved
src/query/src/sql.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.

@irenjj
Copy link
Collaborator Author

irenjj commented Jun 3, 2024

Please add some sqlness tests following https://github.com/GreptimeTeam/greptimedb/blob/main/tests/README.md

It seems that the create flow statement randomly fails.

@discord9
Copy link
Contributor

discord9 commented Jun 4, 2024

Sorry but currently flow only implement for standalone mode, could you move the testcase out of common folder for now? I will put it back when we have distributed support for flow service?

Please add some sqlness tests following https://github.com/GreptimeTeam/greptimedb/blob/main/tests/README.md

It seems that the create flow statement randomly fails.

src/sql/src/statements/show.rs Outdated Show resolved Hide resolved
tests/cases/standalone/show_create_flow.result Outdated Show resolved Hide resolved
irenjj and others added 2 commits June 6, 2024 20:16
@evenyag evenyag requested a review from WenyXu June 7, 2024 02:55
@discord9 discord9 enabled auto-merge June 7, 2024 03:24
@discord9 discord9 added this pull request to the merge queue Jun 7, 2024
Merged via the queue into GreptimeTeam:main with commit 9c42825 Jun 7, 2024
28 checks passed
@irenjj irenjj deleted the show_create_flow branch June 7, 2024 10:50
@tisonkun
Copy link
Collaborator

@irenjj Thanks for your contribution! Would you share your email address to me (by email to tison@greptime.com)?

I'd like to send you a letter for further engagement :D

@irenjj
Copy link
Collaborator Author

irenjj commented Jun 22, 2024

@irenjj Thanks for your contribution! Would you share your email address to me (by email to tison@greptime.com)?

I'd like to send you a letter for further engagement :D

Sure! I've already sent the email.

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.

Implement SHOW CREATE FLOW
6 participants