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

Extend min() and max() to temporal types #1790

Merged
merged 5 commits into from Apr 2, 2024
Merged

Conversation

antepusic
Copy link
Contributor

@antepusic antepusic commented Mar 6, 2024

NB: min() and max() haven’t been extended to Duration values in the interest of compatibility with Neo4j (see Ordering of temporal values in https://neo4j.com/docs/cypher-manual/current/syntax/operators/#cypher-ordering).

[master < Task] PR

  • Check, and update documentation if necessary
  • Provide the full content or a guide for the final git message

    Extend min() and max() methods to work with temporal values

To keep docs changelog up to date, one more thing to do:

  • Write a release note here, including added/changed clauses

    It is now possible to apply the min() and max() methods to temporal values.

  • Tag someone from docs team in the comments

@antepusic antepusic added Ready for review PR is ready for review Docs needed Docs needed labels Mar 6, 2024
@antepusic antepusic added this to the mg-v2.16.0 milestone Mar 6, 2024
@antepusic antepusic self-assigned this Mar 6, 2024
@antepusic
Copy link
Contributor Author

@kgolubic Right now, the min() and max() methods are documented this way: max(row: integer|float) -> (integer|float). Listing all types here would impair readability, how’d you get around this?

@kgolubic
Copy link
Contributor

kgolubic commented Mar 6, 2024

max(row: integer|float) -> (integer|float)

I agree that listing all of the types doesn't look good (1).
We could list temporal and then in the Description list supported types (2)


| Name      | Signature                                               | Description                                                                                           |
|-----------|---------------------------------------------------------|-------------------------------------------------------------------------------------------------------|

| `max`     | `max(row: integer\|float\|Date\|LocalTime\|LocalDateTime) -> (integer\|float\|Date\|LocalTime\|LocalDateTime)`          | Returns the maximum value in a set of values.                                                         |
| `min`     | `min(row: integer\|float\|temporal) -> (integer\|float\|temporal)`          | Returns the minimum value in a set of values.                                                         


image

Copy link
Contributor

@Josipmrden Josipmrden left a comment

Choose a reason for hiding this comment

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

Good to go!

@antepusic antepusic linked an issue Mar 26, 2024 that may be closed by this pull request
@antepusic antepusic requested review from Ignition and removed request for Ignition and gvolfing March 26, 2024 09:26
Copy link

sonarcloud bot commented Apr 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@antepusic antepusic merged commit 2e2a888 into master Apr 2, 2024
7 checks passed
@antepusic antepusic deleted the temporal-min-max branch April 2, 2024 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs needed Docs needed Ready for review PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Min / max functions are not available in Memgraph for date types Add date manipulation functionalities
3 participants