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

Can't define multiple predefined date formats using `||` #2132

Closed
thomasst opened this Issue Aug 2, 2012 · 5 comments

Comments

Projects
None yet
5 participants
@thomasst
Copy link

commented Aug 2, 2012

The following mapping:

'index': {
    'properties': {
        'date_created': {
            'type': 'date',
            'index': 'not_analyzed',
            'format: 'date_time||date_time_no_millis',
        },
    },
}

Gives an error:

ElasticSearchException[Illegal pattern component: t]; nested: IllegalArgumentException[Illegal pattern component: t];

I tried patching as follows, however didn't manage to get it working since forPattern wasn't called as expected:

diff --git a/src/main/java/org/elasticsearch/common/joda/Joda.java b/src/main/java/org/elasticsearch/common/joda/Joda.java
index a4f3427..e8d6e53 100644
--- a/src/main/java/org/elasticsearch/common/joda/Joda.java
+++ b/src/main/java/org/elasticsearch/common/joda/Joda.java
@@ -31,11 +31,11 @@ import org.joda.time.format.*;
  */
 public class Joda {

-    /**
-     * Parses a joda based pattern, including some named ones (similar to the built in Joda ISO ones).
-     */
-    public static FormatDateTimeFormatter forPattern(String input) {
+    private  static DateTimeFormatter formatterForPattern(String input) {
         DateTimeFormatter formatter;
         if ("basicDate".equals(input) || "basic_date".equals(input)) {
             formatter = ISODateTimeFormat.basicDate();
         } else if ("basicDateTime".equals(input) || "basic_date_time".equals(input)) {
@@ -76,9 +76,10 @@ public class Joda {
             formatter = ISODateTimeFormat.dateHourMinuteSecondMillis();
         } else if ("dateOptionalTime".equals(input) || "date_optional_time".equals(input)) {
             // in this case, we have a separate parser and printer since the dataOptionalTimeParser can't print
-            return new FormatDateTimeFormatter(input,
-                    ISODateTimeFormat.dateOptionalTimeParser().withZone(DateTimeZone.UTC),
-                    ISODateTimeFormat.dateTime().withZone(DateTimeZone.UTC));
+            DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder()
+                    .append(ISODateTimeFormat.dateTime().withZone(DateTimeZone.UTC).getPrinter(),
+                            ISODateTimeFormat.dateOptionalTimeParser().withZone(DateTimeZone.UTC).getParser());
+            formatter = builder.toFormatter();
         } else if ("dateTime".equals(input) || "date_time".equals(input)) {
             formatter = ISODateTimeFormat.dateTime();
         } else if ("dateTimeNoMillis".equals(input) || "date_time_no_millis".equals(input)) {
@@ -125,14 +126,27 @@ public class Joda {
                 formatter = DateTimeFormat.forPattern(input);
             } else {
                 DateTimeParser[] parsers = new DateTimeParser[formats.length];
+                DateTimePrinter printer = null;
                 for (int i = 0; i < formats.length; i++) {
-                    parsers[i] = DateTimeFormat.forPattern(formats[i]).withZone(DateTimeZone.UTC).getParser();
+                    DateTimeFormatter f = formatterForPattern(formats[i]);
+                    parsers[i] = f.getParser();
+                    if (i == 0) {
+                        printer = f.getPrinter();
+                    }
                 }
                 DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder()
-                        .append(DateTimeFormat.forPattern(formats[0]).withZone(DateTimeZone.UTC).getPrinter(), parsers);
+                        .append(printer, parsers);
                 formatter = builder.toFormatter();
             }
         }
+        return formatter;
+    }
+
+    /**
+     * Parses a joda based pattern, including some named ones (similar to the built in Joda ISO ones).
+     */
+    public static FormatDateTimeFormatter forPattern(String input) {
+        DateTimeFormatter formatter = formatterForPattern(input);
         return new FormatDateTimeFormatter(input, formatter.withZone(DateTimeZone.UTC));
     }

I'm happy to submit a full patch to make it work, but need a hint on where this exception is actually triggered.

@philfreo

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2012

+1

@kimchy

This comment has been minimized.

Copy link
Member

commented Sep 6, 2012

Yea, the || format is only supported for "explicit" formats, like yyyy||yyyy-MM, not for the "built in" patterns. we can try and support it for built in patters, but for now you can explicitly specify the formats.

@thomasst

This comment has been minimized.

Copy link
Author

commented Sep 6, 2012

@kimchy Any clues on what's missing in the patch?

@ppearcy

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2013

Yeah, it would be very convenient to mix actual formats and Joda descriptions. Reverse engineering what the Joda format strings should be is doable, but inconvenient.

Thanks!

spinscale added a commit to spinscale/elasticsearch that referenced this issue Jun 10, 2013

Allow date format to supported group of built-in patterns
Until now 'named dates' like dateOptionalTime could not be used as a group
of dates. This patch allows it to group it arbitrarily like this:

* yyyy/MM/dd HH:mm:ss||yyyy/MM/dd||dateOptionalTime
* dateOptionalTime||yyyy/MM/dd HH:mm:ss||yyyy/MM/dd
* yyyy/MM/dd HH:mm:ss||dateOptionalTime||yyyy/MM/dd
* date_time||date_time_no_millis

Closes elastic#2132

@spinscale spinscale closed this in 9d3e34b Jun 13, 2013

spinscale added a commit that referenced this issue Jun 13, 2013

Allow date format to supported group of built-in patterns
Until now 'named dates' like dateOptionalTime could not be used as a group
of dates. This patch allows it to group it arbitrarily like this:

* yyyy/MM/dd HH:mm:ss||yyyy/MM/dd||dateOptionalTime
* dateOptionalTime||yyyy/MM/dd HH:mm:ss||yyyy/MM/dd
* yyyy/MM/dd HH:mm:ss||dateOptionalTime||yyyy/MM/dd
* date_time||date_time_no_millis

Closes #2132

mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015

Allow date format to supported group of built-in patterns
Until now 'named dates' like dateOptionalTime could not be used as a group
of dates. This patch allows it to group it arbitrarily like this:

* yyyy/MM/dd HH:mm:ss||yyyy/MM/dd||dateOptionalTime
* dateOptionalTime||yyyy/MM/dd HH:mm:ss||yyyy/MM/dd
* yyyy/MM/dd HH:mm:ss||dateOptionalTime||yyyy/MM/dd
* date_time||date_time_no_millis

Closes elastic#2132
@Hyunsuk-Baek

This comment has been minimized.

Copy link

commented Jan 24, 2017

gg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.