Skip to content
This repository has been archived by the owner on Aug 11, 2020. It is now read-only.

Alow datetime arithemtics via :+ clause and add :interval clause #2

Merged
merged 2 commits into from
Jun 26, 2019

Conversation

sbelak
Copy link
Contributor

@sbelak sbelak commented Jun 26, 2019

This adds support for datetime arithmetic using the :+ clause:

[:+ [:field-id 15] [:interval -1 :month]]

and the accompanying :interval clause.

The idea is that subtraction just gets rewritten by the QP, but I could see an argument for also supporting this in :-.

A further PR could also depreciate :relative-datetime or rather turn it into syntax sugar.

@sbelak sbelak requested a review from camsaul as a code owner June 26, 2019 20:52
Copy link
Member

@camsaul camsaul left a comment

Choose a reason for hiding this comment

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

works for me

@camsaul camsaul merged commit e0f411a into master Jun 26, 2019
@camsaul camsaul deleted the datetime-arithemtics branch June 26, 2019 21:32
ExpressionArg))

(defclause ^{:requires-features #{:expressions}} +
x ExpressionArg, y ExpressionArgOrInterval, more (rest ExpressionArg))
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to not allow intervals for the more args? Or to not allow it for - as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was a bit hesitant about multiple args because date arithemtics is a bit funky and say associativity property doesn't entirely hold. But that might be more a theoretical than practical concern. Happy to add if you reckon.

As for - I suppose it makes more sense to have it, than to strive for maximal parsimony

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants