Review comments: Docs + Allowing negative timestamps

Added docs that only 10/13 length unix timestamps are supported
Added docs in upgrade documentation

Allow for negative timestamps

docs/reference/mapping/date-format.asciidoc

@@ -200,9 +200,14 @@ year.
 year, and two digit day of month.
 
 |`epoch_second`|A formatter for the number of seconds since the epoch.
-
-|`epoch_millis`|A formatter for the number of milliseconds since
-the epoch.
+Note, that this timestamp allows a max length of 10 chars, so dates
+older than 1653 and 2286 are not supported. You should use a different
+date formatter in that case.
+
+|`epoch_millis`|A formatter for the number of milliseconds since the epoch.
+Note, that this timestamp allows a max length of 13 chars, so dates
+older than 1653 and 2286 are not supported. You should use a different
+date formatter in that case.
 |=======================================================================
 
 [float]

docs/reference/migration/migrate_2_0.asciidoc

@@ -286,6 +286,16 @@ Meta fields can no longer be specified within a document. They should be specifi
 via the API.  For example, instead of adding a field `_parent` within a document,
 use the `parent` url parameter when indexing that document.
 
+==== Date format does not support unix timestamps by default
+
+In earlier versions of elasticsearch, every timestamp was always tried to be parsed as
+as unix timestamp first. This means, even when specifying a date format like
+`dateOptionalTime`, one could supply unix timestamps instead of a ISO8601 formatted
+date.
+
+This is not supported anymore. If you want to store unix timestamps, you need to specify
+the appropriate formats in the mapping, namely `epoch_second` or `epoch_millis`.
+
 ==== Source field limitations
 The `_source` field could previously be disabled dynamically. Since this field
 is a critical piece of many features like the Update API, it is no longer
@@ -641,4 +651,4 @@ The following breaks in backwards compatability have been made on indices with t
 created on or after clusters with version 2.0:
 * The `type` option on the `_parent` field can only point to a parent type that doesn't exist yet,
   so this means that an existing type/mapping can no longer become a parent type.
-* The `has_child` and `has_parent` queries can no longer be use in alias filters.
+* The `has_child` and `has_parent` queries can no longer be use in alias filters.

src/main/java/org/elasticsearch/common/joda/Joda.java

@@ -26,6 +26,8 @@
 import org.joda.time.field.ScaledDurationField;
 import org.joda.time.format.*;
 
+import java.io.IOException;
+import java.io.Writer;
 import java.util.Locale;
 import java.util.regex.Pattern;
 
@@ -135,9 +137,9 @@ public static FormatDateTimeFormatter forPattern(String input, Locale locale) {
         } else if ("yearMonthDay".equals(input) || "year_month_day".equals(input)) {
             formatter = ISODateTimeFormat.yearMonthDay();
         } else if ("epoch_second".equals(input)) {
-            formatter = new DateTimeFormatterBuilder().append(new EpochTimeParser(false)).toFormatter();
+            formatter = new DateTimeFormatterBuilder().append(forPattern("dateOptionalTime").printer().getPrinter(), new EpochTimeParser(false)).toFormatter();
         } else if ("epoch_millis".equals(input)) {
-            formatter = new DateTimeFormatterBuilder().append(new EpochTimeParser(true)).toFormatter();
+            formatter = new DateTimeFormatterBuilder().append(forPattern("dateOptionalTime").printer().getPrinter(), new EpochTimeParser(true)).toFormatter();
         } else if (Strings.hasLength(input) && input.contains("||")) {
                 String[] formats = Strings.delimitedListToStringArray(input, "||");
                 DateTimeParser[] parsers = new DateTimeParser[formats.length];
@@ -200,8 +202,8 @@ public DateTimeField getField(Chronology chronology) {
 
     public static class EpochTimeParser implements DateTimeParser {
 
-        private static final Pattern MILLI_SECOND_PRECISION_PATTERN = Pattern.compile("^-\\d+|\\d{1,13}$");
-        private static final Pattern SECOND_PRECISION_PATTERN = Pattern.compile("^-\\d+|\\d{1,10}$");
+        private static final Pattern MILLI_SECOND_PRECISION_PATTERN = Pattern.compile("^-?\\d{1,13}$");
+        private static final Pattern SECOND_PRECISION_PATTERN = Pattern.compile("^-?\\d{1,10}$");
 
         private final boolean hasMilliSecondPrecision;
         private final Pattern pattern;
@@ -245,5 +247,5 @@ public int parseInto(DateTimeParserBucket bucket, String text, int position) {
             }
             return text.length();
         }
-    };
+    }
 }

src/test/java/org/elasticsearch/deps/joda/SimpleJodaTests.java

@@ -274,13 +274,13 @@ public void testThatEpochsCanBeParsed() {
     @Test
     public void testThatNegativeEpochsCanBeParsed() {
         // problem: negative epochs can be arbitrary in size...
-        boolean parseMilliSeconds = true;
+        boolean parseMilliSeconds = randomBoolean();
         FormatDateTimeFormatter formatter = Joda.forPattern(parseMilliSeconds ? "epoch_millis" : "epoch_second");
         DateTime dateTime = formatter.parser().parseDateTime("-10000");
 
         assertThat(dateTime.getYear(), is(1969));
-        assertThat(dateTime.getDayOfMonth(), is(31));
         assertThat(dateTime.getMonthOfYear(), is(12));
+        assertThat(dateTime.getDayOfMonth(), is(31));
         if (parseMilliSeconds) {
             assertThat(dateTime.getHourOfDay(), is(23)); // utc timezone, +2 offset due to CEST
             assertThat(dateTime.getMinuteOfHour(), is(59));
@@ -292,11 +292,14 @@ public void testThatNegativeEpochsCanBeParsed() {
         }
 
         // every negative epoch must be parsed, no matter if exact the size or bigger
-        formatter.parser().parseDateTime("-100000000");
-        formatter.parser().parseDateTime("-100000000000");
-        formatter.parser().parseDateTime("-10000000000000");
-        formatter.parser().parseDateTime("-1000000000001");
-        formatter.parser().parseDateTime("-999999999999");
+        if (parseMilliSeconds) {
+            formatter.parser().parseDateTime("-100000000");
+            formatter.parser().parseDateTime("-999999999999");
+            formatter.parser().parseDateTime("-1234567890123");
+        } else {
+            formatter.parser().parseDateTime("-100000000");
+            formatter.parser().parseDateTime("-1234567890");
+        }
     }
 
     @Test(expected = IllegalArgumentException.class)
@@ -311,6 +314,12 @@ public void testForInvalidDatesInEpochMillis() {
         formatter.parser().parseDateTime(randomFrom("invalid date", "12345678901234"));
     }
 
+    public void testThatEpochParserIsPrinter() {
+        FormatDateTimeFormatter formatter = Joda.forPattern("epoch_millis");
+        assertThat(formatter.parser().isPrinter(), is(true));
+        assertThat(formatter.printer().isPrinter(), is(true));
+    }
+
     private long utcTimeInMillis(String time) {
         return ISODateTimeFormat.dateOptionalTimeParser().withZone(DateTimeZone.UTC).parseMillis(time);
     }