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

Add set_system_time function #32

Merged
merged 10 commits into from
Sep 20, 2023
Merged

Add set_system_time function #32

merged 10 commits into from
Sep 20, 2023

Conversation

radomird
Copy link
Contributor

@radomird radomird commented Jan 10, 2022

Closes #31

I've added a new file with set_system_time function that will set a configuration parameter on the db called user_defined.system_time and fill in the desired value. The versioning function will then try to use this config parameter to set the time_stamp_to_use.

  • added new function ✅
  • updated versioning_function and versioning_function_no_checks too use the new config parameter ✅
  • updated README ✅
  • add tests ✅

@radomird radomird requested a review from paolochiodi January 10, 2022 16:04
@radomird
Copy link
Contributor Author

Hi @paolochiodi I've tried to implement the requested missing feature. Could you please take a look if I'm heading in the right direction?

Thanks.

@radomird radomird self-assigned this Jan 11, 2022
@radomird radomird marked this pull request as ready for review January 11, 2022 13:48
@simoneb
Copy link
Member

simoneb commented May 11, 2022

@paolochiodi can you please take a look at this PR? we forgot about it

@dzolo
Copy link
Contributor

dzolo commented May 11, 2022

Nice missing feature. I would suggest to use datetime format YYYY-MM-DD HH24:MI:SS.US in order to be capable to set precise time.

It would be also nice to have timezone support (as in original support) but this will be more work to be done as TO_TIMESTAMP does not support TZ. Maybe cast to timestamp with time zone could be used instead of TO_TIMESTAMP?

@radomird
Copy link
Contributor Author

radomird commented Sep 19, 2023

@dzolo thanks for the suggestion, adding ms portion to the timestamp is not a bad idea. I will update the date/time format.

Regarding the Timezone support - would be a nice addition, however, I would probably open a feature request issue and that can be tackled in a separate PR, in order not to make this PR more complex than it needs to be.

@radomird
Copy link
Contributor Author

radomird commented Sep 20, 2023

Support for milliseconds and microseconds added. They are both optional.

@dzolo a review of the PR would be appreciated 😉

@radomird radomird requested review from simoneb and removed request for paolochiodi September 20, 2023 10:41
package.json Outdated Show resolved Hide resolved
versioning_function.sql Outdated Show resolved Hide resolved
@radomird radomird requested a review from simoneb September 20, 2023 11:01
Copy link
Member

@simoneb simoneb left a comment

Choose a reason for hiding this comment

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

a couple of comments but overall LGTM, nice job 👌

README.md Outdated Show resolved Hide resolved
@radomird radomird merged commit 1dac089 into master Sep 20, 2023
12 checks passed
@radomird radomird deleted the feat/set_system_time branch September 20, 2023 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set_system_time is not supported
3 participants