Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/581#discussion_r95887588
  
    --- Diff: 
logical/src/main/java/org/apache/drill/common/expression/fn/JodaDateValidator.java
 ---
    @@ -0,0 +1,216 @@
    +/*
    +* Licensed to the Apache Software Foundation (ASF) under one or more
    +* contributor license agreements.  See the NOTICE file distributed with
    +* this work for additional information regarding copyright ownership.
    +* The ASF licenses this file to you under the Apache License, Version 2.0
    +* (the "License"); you may not use this file except in compliance with
    +* the License.  You may obtain a copy of the License at
    +*
    +* http://www.apache.org/licenses/LICENSE-2.0
    +*
    +* Unless required by applicable law or agreed to in writing, software
    +* distributed under the License is distributed on an "AS IS" BASIS,
    +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +* See the License for the specific language governing permissions and
    +* limitations under the License.
    +*/
    +package org.apache.drill.common.expression.fn;
    +
    +import com.google.common.collect.Sets;
    +import org.apache.commons.lang3.StringUtils;
    +import org.apache.drill.common.map.CaseInsensitiveMap;
    +
    +import java.util.Comparator;
    +import java.util.Set;
    +
    +public class JodaDateValidator {
    +
    +  private static final Set<String> postgresValuesForDeleting = 
Sets.newTreeSet(new LengthDescComparator());
    +  private static final CaseInsensitiveMap<String> postgresToJodaMap = 
CaseInsensitiveMap.newTreeMap(new LengthDescComparator());
    +
    +  // tokens for deleting
    +  public static final String SUFFIX_SP = "sp";
    +  public static final String PREFIX_FM = "fm";
    +  public static final String PREFIX_FX = "fx";
    +  public static final String PREFIX_TM = "tm";
    +
    +  // postgres patterns
    +  public static final String POSTGRES_FULL_NAME_OF_DAY = "day";
    +  public static final String POSTGRES_DAY_OF_YEAR = "ddd";
    +  public static final String POSTGRES_DAY_OF_MONTH = "dd";
    +  public static final String POSTGRES_DAY_OF_WEEK = "d";
    +  public static final String POSTGRES_NAME_OF_MONTH = "month";
    +  public static final String POSTGRES_ABR_NAME_OF_MONTH = "mon";
    +  public static final String POSTGRES_FULL_ERA_NAME = "ee";
    +  public static final String POSTGRES_NAME_OF_DAY = "dy";
    +  public static final String POSTGRES_TIME_ZONE_NAME = "tz";
    +  public static final String POSTGRES_HOUR_12_NAME = "hh";
    +  public static final String POSTGRES_HOUR_12_OTHER_NAME = "hh12";
    +  public static final String POSTGRES_HOUR_24_NAME = "hh24";
    +  public static final String POSTGRES_MINUTE_OF_HOUR_NAME = "mi";
    +  public static final String POSTGRES_SECOND_OF_MINUTE_NAME = "ss";
    +  public static final String POSTGRES_MILLISECOND_OF_MINUTE_NAME = "ms";
    +  public static final String POSTGRES_WEEK_OF_YEAR = "ww";
    +  public static final String POSTGRES_MONTH = "mm";
    +  public static final String POSTGRES_HALFDAY_AM = "am";
    +  public static final String POSTGRES_HALFDAY_PM = "pm";
    +
    +  // jodaTime patterns
    +  public static final String JODA_FULL_NAME_OF_DAY = "EEEE";
    +  public static final String JODA_DAY_OF_YEAR = "D";
    +  public static final String JODA_DAY_OF_MONTH = "d";
    +  public static final String JODA_DAY_OF_WEEK = "e";
    +  public static final String JODA_NAME_OF_MONTH = "MMMM";
    +  public static final String JODA_ABR_NAME_OF_MONTH = "MMM";
    +  public static final String JODA_FULL_ERA_NAME = "G";
    +  public static final String JODA_NAME_OF_DAY = "E";
    +  public static final String JODA_TIME_ZONE_NAME = "TZ";
    +  public static final String JODA_HOUR_12_NAME = "h";
    +  public static final String JODA_HOUR_12_OTHER_NAME = "h";
    +  public static final String JODA_HOUR_24_NAME = "H";
    +  public static final String JODA_MINUTE_OF_HOUR_NAME = "m";
    +  public static final String JODA_SECOND_OF_MINUTE_NAME = "s";
    +  public static final String JODA_MILLISECOND_OF_MINUTE_NAME = "SSS";
    +  public static final String JODA_WEEK_OF_YEAR = "w";
    +  public static final String JODA_MONTH = "MM";
    +  public static final String JODA_HALFDAY = "aa";
    +
    +  static {
    +    postgresToJodaMap.put(POSTGRES_FULL_NAME_OF_DAY, 
JODA_FULL_NAME_OF_DAY);
    +    postgresToJodaMap.put(POSTGRES_DAY_OF_YEAR, JODA_DAY_OF_YEAR);
    +    postgresToJodaMap.put(POSTGRES_DAY_OF_MONTH, JODA_DAY_OF_MONTH);
    +    postgresToJodaMap.put(POSTGRES_DAY_OF_WEEK, JODA_DAY_OF_WEEK);
    +    postgresToJodaMap.put(POSTGRES_NAME_OF_MONTH, JODA_NAME_OF_MONTH);
    +    postgresToJodaMap.put(POSTGRES_ABR_NAME_OF_MONTH, 
JODA_ABR_NAME_OF_MONTH);
    +    postgresToJodaMap.put(POSTGRES_FULL_ERA_NAME, JODA_FULL_ERA_NAME);
    +    postgresToJodaMap.put(POSTGRES_NAME_OF_DAY, JODA_NAME_OF_DAY);
    +    postgresToJodaMap.put(POSTGRES_TIME_ZONE_NAME, JODA_TIME_ZONE_NAME);
    +    postgresToJodaMap.put(POSTGRES_HOUR_12_NAME, JODA_HOUR_12_NAME);
    +    postgresToJodaMap.put(POSTGRES_HOUR_12_OTHER_NAME, 
JODA_HOUR_12_OTHER_NAME);
    +    postgresToJodaMap.put(POSTGRES_HOUR_24_NAME, JODA_HOUR_24_NAME);
    +    postgresToJodaMap.put(POSTGRES_MINUTE_OF_HOUR_NAME, 
JODA_MINUTE_OF_HOUR_NAME);
    +    postgresToJodaMap.put(POSTGRES_SECOND_OF_MINUTE_NAME, 
JODA_SECOND_OF_MINUTE_NAME);
    +    postgresToJodaMap.put(POSTGRES_MILLISECOND_OF_MINUTE_NAME, 
JODA_MILLISECOND_OF_MINUTE_NAME);
    +    postgresToJodaMap.put(POSTGRES_WEEK_OF_YEAR, JODA_WEEK_OF_YEAR);
    +    postgresToJodaMap.put(POSTGRES_MONTH, JODA_MONTH);
    +    postgresToJodaMap.put(POSTGRES_HALFDAY_AM, JODA_HALFDAY);
    +    postgresToJodaMap.put(POSTGRES_HALFDAY_PM, JODA_HALFDAY);
    +  }
    +
    +  static {
    +    postgresValuesForDeleting.add(SUFFIX_SP);
    +    postgresValuesForDeleting.add(PREFIX_FM);
    +    postgresValuesForDeleting.add(PREFIX_FX);
    +    postgresValuesForDeleting.add(PREFIX_TM);
    +  }
    +
    +  /**
    +   * Replaces all postgres patterns from {@param pattern},
    +   * available in postgresToJodaMap keys to jodaTime equivalents.
    +   *
    +   * @param pattern date pattern in postgres format
    +   * @return date pattern with replaced patterns in joda format
    +   */
    +  public static String toJodaFormat(String pattern) {
    --- End diff --
    
    The premise of this function is that no one pattern is a subset of another 
pattern. And, no one pattern is the same as the same as part of two adjacent 
patterns. That is, we know that, say, SSSS is a single pattern and not two 
copies of SS next to one another. Have we done the analysis to ensure that no 
such aliases are possible?
    
    Or, for example, consider these patterns: DAY, Y, YY, YYY, IYY and IYYY. If 
we replace "Y" first, we end up matching "DAY" or "IYY" when we should not. If 
we do "IY" first, we miss "IYYY". If the input is "DAYY", and we do "YY" first, 
we miss that this is "DAY - Y", not "DA - YY". In short, context seems 
essential. Or, the substitutions must be carefully ordered. I think that, for 
the above, "IYYY", "IYY", "YYY", "YY", "DAY", "Y" would be the proper order. 
But, since we use a map, do we have an implied replacement order?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to