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

Date/time support for Postgres. Year/month/day operations on Columns. #6153

Merged
merged 15 commits into from
Mar 31, 2023

Conversation

radeusgd
Copy link
Member

@radeusgd radeusgd commented Mar 30, 2023

Pull Request Description

Closes #6115

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.


example_should_start_with = "Hello World!" . should_start_with "Hello"
Error.should_start_with : Any -> Integer -> Test_Result
Error.should_start_with self _ frames_to_skip=0 = Test.fail_match_on_unexpected_error self 1+frames_to_skip
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this always fail?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it is called on an error value and a non-error was expected.

This is due to a slightly weird calling convention.

If I call a method on a dataflow error, by default it is not called but the dataflow error is "propagated" - so it is just returned as-is. So - if I define an extension method on Any, it will resolve on any value, but it will return dataflow errors unchanged if called on it. In fact you can call (Error.throw "X").sgdjkkgjkdgjkdfkjgfjk and this will be a valid call and will return the Error "X" - that is because if we are holding an error, there's no way to tell what "would be" instead of it if there wasn't an error and what methods could that have - so all methods are allowed and they just act as identity.

But sometimes, I don't want the call to just fall through. To avoid this, I can explicitly add an extension method on the Error type. Then such methods will take precedence on error instances and allow me to customize the behaviour.

So in our Test suite, it is customary to define every extension method both on Any and on Error. That way if a value returns an Error, we can get an information that a test failed. It is very important because if you do not define it on Error, then a call (Error.throw "X").should_equal "Y" would fall-through and return the dataflow error - but if it's result is nowhere used - that will just be silently ignored - so a test will pass even if it's wrong - that's very bad for a test suite.

Comment on lines -287 to -296
## Format this time of day as text using the default formatter.

> Example
Convert the current time to text.

from Standard.Base import Time_Of_Day

example_to_text = Time_Of_Day.now.to_text
to_text : Text
to_text self = @Builtin_Method "Time_Of_Day.to_text"
Copy link
Member Author

Choose a reason for hiding this comment

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

Other date/time types do not have this.

Having it breaks the to_text invocation on type.

I.e. Time_Of_Day.to_text now works. With that shadow definition it wasn't working properly.

It's a bit of a workaround to #1538

@radeusgd radeusgd force-pushed the wip/radeusgd/postgres-dates-6115 branch from ae4726b to 1832d76 Compare March 31, 2023 14:14
@farmaazon farmaazon removed their request for review March 31, 2023 14:19
Base automatically changed from wip/radeusgd/value-types-1 to develop March 31, 2023 16:16
… mostly portable (can always be overridden if necessary).
… making debugging easier. Also print if warnings are attached."

This reverts commit 20318ee.
@radeusgd radeusgd force-pushed the wip/radeusgd/postgres-dates-6115 branch from 1832d76 to b03a30c Compare March 31, 2023 16:22
Copy link
Member

@jdunkerley jdunkerley left a comment

Choose a reason for hiding this comment

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

Couple of nits. But LGTM

@radeusgd radeusgd added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Mar 31, 2023
@radeusgd radeusgd added the CI: Ready to merge This PR is eligible for automatic merge label Mar 31, 2023
@mergify mergify bot merged commit 6ddcb55 into develop Mar 31, 2023
@mergify mergify bot deleted the wip/radeusgd/postgres-dates-6115 branch March 31, 2023 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
4 participants