-
Notifications
You must be signed in to change notification settings - Fork 328
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 statement/execution timeout session variable #4792
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
a0d7cb1
to
4cb763e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4792 +/- ##
==========================================
- Coverage 84.05% 83.74% -0.31%
==========================================
Files 1142 1144 +2
Lines 211545 213654 +2109
==========================================
+ Hits 177804 178929 +1125
- Misses 33741 34725 +984 |
Nice work 👍 The input with time unit of PostgreSQL seems not implemented, would you like to implement it in this PR? Also, it would be better to add time unit while outputting the variable value to the client. Here is an example in PostgreSQL:
|
Thank you for the feedbacks, I added an implementation of postgres time unit support in the newer iteration of the pr. |
c0b07c0
to
8582e1f
Compare
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.
Please resolve the conflicts. You might need to merge the latest main branch.
2bbf954
to
8549044
Compare
f76f686
to
58765fb
Compare
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.
Others LGTM
Good job! Is it ready for review? @lyang24 |
1eea496
to
dec9e08
Compare
dec9e08
to
379f72b
Compare
379f72b
to
aa7c5e3
Compare
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
#4760
What's changed and what's your intention?
Summarize your change (mandatory):
This pr implements session variables for query timeout - the show and set dialects are supported.
How does this PR work? Need a brief introduction for the changed logic (optional)
The query timeouts are per session and its implemented with applying a tokio timeout to the query processing function
Checklist