Allow min max to work with dates#152
Closed
jva wants to merge 9 commits into
Closed
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends Mondrian’s Min/Max behavior to support DateTime expressions (including correct format-string inference), and adds integration/spec coverage to validate date and numeric behavior across common query patterns (Filter, IIf, nesting, etc.).
Changes:
- Add a
FormatAwareFunDefopt-in mechanism so functions/UDF adapters can explicitly control which argument drives FORMAT_STRING inference. - Update
Min/Maximplementation and format inference logic to handle DateTime results correctly (including in the presence ofFilter). - Add RSpec coverage for date-based
Min/Maxbehavior and a few related edge cases/regressions.
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| mondrian/pom.xml | Build/test lifecycle changes to support IT sources/resources and updated plugin/config behavior. |
| mondrian/src/it/java/mondrian/rolap/cache/SegmentCacheIndexImplTest.java | Adjust test construction for updated SegmentCacheIndexImpl API (thread array). |
| mondrian/src/main/java/mondrian/mdx/ResolvedFunCall.java | Exposes FormatAwareFunDef capability from resolved function calls. |
| mondrian/src/main/java/mondrian/olap/FormatAwareFunDef.java | New interface allowing functions/UDFs to control format-string inference source. |
| mondrian/src/main/java/mondrian/olap/Formula.java | Uses FormatAwareFunDef (when present) to select format inheritance argument. |
| mondrian/src/main/java/mondrian/olap/fun/MinMaxFunDef.java | Extends Min/Max runtime evaluation to handle DateTime values and participates in format inference. |
| pom.xml | Updates dependency versions / compiler properties used by the overall build. |
| spec/mondrian_spec.rb | Adds specs covering DateTime and numeric Min/Max, including formatting and edge cases. |
You can also share your feedback on Copilot code review. Take the survey.
Owner
|
Create pull request in the new https://github.com/rsim/mondrian-olap-java repo. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Min/Max now work with dates.
Tried to create a comprehensive coverage of tests, as well as for failing edge cases that were discovered during development. The pom.xml changes are for compilation for a person that does not have a cache of the needed Maven artifacts. Also for running the Mondrian's test suite with
mvn verify -DrunITs. That helped identify a few initial regressions.The added
FormatAwareFunDefis also a step in the direction of UDFs being able to opt in and tell the caller their type instead of relying on existing check which scans the params depth-first and finds the first argument with known type. This will be used later, e.g. -DateDiffDayshas two date parameters, but the default formatting of the return value will be numeric instead of currently detected date.