Skip to content

Commit c8e4824

Browse files
authored
Merge pull request #2256 from elastic/fix/date-math-fractional-time
fix #2170, DateMath should not serialize time as fractional
2 parents 311e13f + 8c19419 commit c8e4824

File tree

3 files changed

+86
-21
lines changed

3 files changed

+86
-21
lines changed

src/Nest/CommonOptions/DateMath/DateMath.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ public override string ToString()
9494
foreach (var r in Self.Ranges)
9595
{
9696
sb.Append(r.Item1.GetStringValue());
97-
sb.Append(r.Item2);
97+
//date math does not support fractional time units so e.g TimeSpan.FromHours(25) should not yield 1.04d
98+
sb.Append(Time.ToFirstUnitYieldingInteger(r.Item2));
9899
}
99100
if (Self.Round.HasValue)
100101
sb.Append("/" + Self.Round.Value.GetStringValue());
@@ -120,4 +121,4 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
120121
}
121122
}
122123
}
123-
}
124+
}

src/Nest/CommonOptions/TimeUnit/Time.cs

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,43 @@ public int CompareTo(Time other)
7272
return 1;
7373
}
7474

75+
public static Time ToFirstUnitYieldingInteger(Time fractionalTime)
76+
{
77+
var fraction = fractionalTime.Factor.GetValueOrDefault(double.Epsilon);
78+
if (IsIntegerGreaterThen0(fraction)) return fractionalTime;
79+
80+
var ms = fractionalTime.Milliseconds;
81+
if (ms > _week)
82+
{
83+
fraction = ms / _week;
84+
if (IsIntegerGreaterThen0(fraction)) return new Time(fraction, TimeUnit.Week);
85+
}
86+
if (ms > _day)
87+
{
88+
fraction = ms / _day;
89+
if (IsIntegerGreaterThen0(fraction)) return new Time(fraction, TimeUnit.Day);
90+
}
91+
if (ms > _hour)
92+
{
93+
fraction = ms / _hour;
94+
if (IsIntegerGreaterThen0(fraction)) return new Time(fraction, TimeUnit.Hour);
95+
}
96+
if (ms > _minute)
97+
{
98+
fraction = ms / _minute;
99+
if (IsIntegerGreaterThen0(fraction)) return new Time(fraction, TimeUnit.Minute);
100+
}
101+
if (ms > _second)
102+
{
103+
fraction = ms / _second;
104+
if (IsIntegerGreaterThen0(fraction)) return new Time(fraction, TimeUnit.Second);
105+
}
106+
return new Time(ms, TimeUnit.Millisecond);
107+
}
108+
109+
private static bool IsIntegerGreaterThen0(double d) => Math.Abs(d % 1) < double.Epsilon;
110+
111+
75112
public static bool operator <(Time left, Time right) => left.CompareTo(right) < 0;
76113
public static bool operator <=(Time left, Time right) => left.CompareTo(right) < 0 || left.Equals(right);
77114

@@ -81,7 +118,7 @@ public int CompareTo(Time other)
81118
public static bool operator ==(Time left, Time right) =>
82119
ReferenceEquals(left, null) ? ReferenceEquals(right, null) : left.Equals(right);
83120

84-
public static bool operator !=(Time left, Time right) => !(left == right);
121+
public static bool operator !=(Time left, Time right) => !(left == right);
85122

86123
public TimeSpan ToTimeSpan() => TimeSpan.FromMilliseconds(this.Milliseconds);
87124

@@ -190,4 +227,4 @@ private void Reduce(double ms)
190227
}
191228
}
192229
}
193-
}
230+
}

src/Tests/CommonOptions/DateMath/DateMathExpressions.doc.cs

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,32 +10,32 @@ public class DateMathEpressions
1010
{
1111
/** == Date Math Expressions
1212
* The date type supports using date math expression when using it in a query/filter
13-
* Whenever durations need to be specified, eg for a timeout parameter, the duration can be specified
13+
* Whenever durations need to be specified, eg for a timeout parameter, the duration can be specified
1414
*
15-
* The expression starts with an "anchor" date, which can be either now or a date string (in the applicable format) ending with `||`.
16-
* It can then follow by a math expression, supporting `+`, `-` and `/` (rounding).
17-
* The units supported are
15+
* The expression starts with an "anchor" date, which can be either now or a date string (in the applicable format) ending with `||`.
16+
* It can then follow by a math expression, supporting `+`, `-` and `/` (rounding).
17+
* The units supported are
1818
*
1919
* - `y` (year)
2020
* - `M` (month)
21-
* - `w` (week)
22-
* - `d` (day)
23-
* - `h` (hour)
21+
* - `w` (week)
22+
* - `d` (day)
23+
* - `h` (hour)
2424
* - `m` (minute)
2525
* - `s` (second)
26-
*
27-
* as a whole number representing time in milliseconds, or as a time value like `2d` for 2 days.
26+
*
27+
* as a whole number representing time in milliseconds, or as a time value like `2d` for 2 days.
2828
* :datemath: {ref_current}/common-options.html#date-math
2929
* Be sure to read the Elasticsearch documentation on {datemath}[Date Math].
3030
*/
3131
[U] public void SimpleExpressions()
3232
{
3333
/** === Simple Expressions
34-
* You can create simple expressions using any of the static methods on `DateMath`
34+
* You can create simple expressions using any of the static methods on `DateMath`
3535
*/
3636
Expect("now").WhenSerializing(Nest.DateMath.Now);
3737
Expect("2015-05-05T00:00:00").WhenSerializing(Nest.DateMath.Anchored(new DateTime(2015,05, 05)));
38-
38+
3939
/** strings implicitly convert to `DateMath` */
4040
Expect("now").WhenSerializing<Nest.DateMath>("now");
4141

@@ -46,18 +46,18 @@ [U] public void SimpleExpressions()
4646
/** the resulting date math will assume the whole string is the anchor */
4747
.Result(dateMath => ((IDateMath)dateMath)
4848
.Anchor.Match(
49-
d => d.Should().NotBe(default(DateTime)),
49+
d => d.Should().NotBe(default(DateTime)),
5050
s => s.Should().Be(nonsense)
5151
)
5252
);
53-
53+
5454
/** `DateTime` also implicitly convert to simple date math expressions */
5555
var date = new DateTime(2015, 05, 05);
5656
Expect("2015-05-05T00:00:00").WhenSerializing<Nest.DateMath>(date)
5757
/** the anchor will be an actual `DateTime`, even after a serialization/deserialization round trip */
5858
.Result(dateMath => ((IDateMath)dateMath)
5959
. Anchor.Match(
60-
d => d.Should().Be(date),
60+
d => d.Should().Be(date),
6161
s => s.Should().BeNull()
6262
)
6363
);
@@ -66,7 +66,7 @@ . Anchor.Match(
6666
[U] public void ComplexExpressions()
6767
{
6868
/** === Complex Expressions
69-
* Ranges can be chained on to simple expressions
69+
* Ranges can be chained on to simple expressions
7070
*/
7171
Expect("now+1d").WhenSerializing(
7272
Nest.DateMath.Now.Add("1d"));
@@ -80,15 +80,42 @@ [U] public void ComplexExpressions()
8080
Nest.DateMath.Now.Add("1d")
8181
.Subtract(TimeSpan.FromMinutes(1))
8282
.RoundTo(Nest.TimeUnit.Day));
83-
83+
8484
/** When anchoring dates, a `||` needs to be appended as clear separator between the anchor and ranges.
85-
* Again, multiple ranges can be chained
85+
* Again, multiple ranges can be chained
8686
*/
8787
Expect("2015-05-05T00:00:00||+1d-1m").WhenSerializing(
8888
Nest.DateMath.Anchored(new DateTime(2015,05,05))
8989
.Add("1d")
9090
.Subtract(TimeSpan.FromMinutes(1)));
9191
}
9292

93+
[U] public void FractionalsUnitsAreDroppeToIntegerPart()
94+
{
95+
/** === Fractional times
96+
* DateMath expressions do not support fractional numbers so unlike `Time` DateMath will
97+
* pick the biggest integer unit it can represent
98+
*/
99+
Expect("now+25h").WhenSerializing(
100+
Nest.DateMath.Now.Add(TimeSpan.FromHours(25)));
101+
102+
/** where as `Time` on its own serializes like this */
103+
Expect("1.04d").WhenSerializing(new Time(TimeSpan.FromHours(25)));
104+
105+
Expect("now+90001s").WhenSerializing(
106+
Nest.DateMath.Now.Add(TimeSpan.FromHours(25).Add(TimeSpan.FromSeconds(1))));
107+
108+
Expect("now+90000001ms").WhenSerializing(
109+
Nest.DateMath.Now.Add(TimeSpan.FromHours(25).Add(TimeSpan.FromMilliseconds(1))));
110+
111+
Expect("now+1y").WhenSerializing(
112+
Nest.DateMath.Now.Add("1y"));
113+
114+
Expect("now+52w").WhenSerializing(
115+
Nest.DateMath.Now.Add(TimeSpan.FromDays(7 * 52)));
116+
117+
}
118+
119+
93120
}
94121
}

0 commit comments

Comments
 (0)