-
Notifications
You must be signed in to change notification settings - Fork 53
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
chore: Postgres enhance get oldest timestamp #2687
chore: Postgres enhance get oldest timestamp #2687
Conversation
This PR may contain changes to database schema of one of the drivers. If you are introducing any changes to the schema, make sure the upgrade from the latest release to this change passes without any errors/issues. Please make sure the label |
You can find the image built from this PR at
Built from 73b4758 |
You can find the image built from this PR at
Built from 73b4758 |
ba19625
to
ea24341
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.
LGTM thanks!
@@ -701,7 +701,10 @@ proc getInt( | |||
try: | |||
retInt = parseInt(str) | |||
except ValueError: | |||
return err("exception in getInt, parseInt: " & getCurrentExceptionMsg()) | |||
return err( | |||
fmt"exception in getInt, parseInt, str:[{str}] query:[{query}]: " & |
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.
FYI fmt
can raise and prevent using raises: []
on procs.
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.
FYI
fmt
can raise and prevent usingraises: []
on procs.
Thanks for the comment! I fixed that even though I cannot link to the actual commit (I had to force-push
as I rebased from origin.)
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.
LGTM!
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.
Thank you!
ea24341
to
784f856
Compare
* postgres: consider also the existing paritions when getting oldest timestamp * test_driver_postgres_query: adapt test to oldest timestamp
Description
This PR enhances the case where, for any reason, there are old empty partitions. In that case of empty partitions, they are not considered when getting the oldest timestamp. Instead, the oldest timestamp was only considered from the oldest row.
Therefore, when "time" retention policy is applied, we should also consider those old-empty partitions so that they can get erased properly.
Issue
closes #2689