Github user fsparv commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/304#discussion_r161122011
  
    --- Diff: solr/core/src/java/org/apache/solr/cloud/CreateAliasCmd.java ---
    @@ -45,13 +147,92 @@ public CreateAliasCmd(OverseerCollectionMessageHandler 
ocmh) {
       public void call(ClusterState state, ZkNodeProps message, NamedList 
results)
           throws Exception {
         final String aliasName = message.getStr(NAME);
    -    final List<String> canonicalCollectionList = 
parseCollectionsParameter(message.get("collections"));
    -    final String canonicalCollectionsString = 
StrUtils.join(canonicalCollectionList, ',');
    -
         ZkStateReader zkStateReader = ocmh.zkStateReader;
    -    validateAllCollectionsExistAndNoDups(canonicalCollectionList, 
zkStateReader);
    +    ZkStateReader.AliasesManager holder = zkStateReader.aliasesHolder;
    +    if (!anyRoutingParams(message)) {
    +      final List<String> canonicalCollectionList = 
parseCollectionsParameter(message.get("collections"));
    +      final String canonicalCollectionsString = 
StrUtils.join(canonicalCollectionList, ',');
    +      validateAllCollectionsExistAndNoDups(canonicalCollectionList, 
zkStateReader);
    +      holder.applyModificationAndExportToZk(aliases -> 
aliases.cloneWithCollectionAlias(aliasName, canonicalCollectionsString));
    +    } else {
    +      final String routedField = message.getStr(ROUTING_FIELD);
    +      final String routingType = message.getStr(ROUTING_TYPE);
    +      final String tz = message.getStr(TZ);
    +      final String start = message.getStr(START);
    +      final String increment = message.getStr(ROUTING_INCREMENT);
    +      final String maxFutureMs = message.getStr(ROUTING_MAX_FUTURE);
    +
    +      try {
    +        if (0 > Long.valueOf(maxFutureMs)) {
    +          throw new NumberFormatException("Negative value not allowed 
here");
    +        }
    +      } catch (NumberFormatException e) {
    +        throw new SolrException(BAD_REQUEST, ROUTING_MAX_FUTURE + " must 
be a valid long integer representing a number " +
    +            "of milliseconds greater than or equal to zero");
    +      }
     
    -    zkStateReader.aliasesHolder.applyModificationAndExportToZk(aliases -> 
aliases.cloneWithCollectionAlias(aliasName, canonicalCollectionsString));
    +      // Validate we got everything we need
    +      if (routedField == null || routingType == null || start == null || 
increment == null) {
    +        SolrException solrException = new SolrException(BAD_REQUEST, "If 
any of " + CREATE_ROUTED_ALIAS_PARAMS +
    +            " are supplied, then all of " + REQUIRED_ROUTING_PARAMS + " 
must be present.");
    +        log.error("Could not create routed alias",solrException);
    +        throw solrException;
    +      }
    +
    +      if (!"time".equals(routingType)) {
    +        SolrException solrException = new SolrException(BAD_REQUEST, "Only 
time based routing is supported at this time");
    +        log.error("Could not create routed alias",solrException);
    +        throw solrException;
    +      }
    +      // Check for invalid timezone
    +      if(tz != null && !TimeZoneUtils.KNOWN_TIMEZONE_IDS.contains(tz)) {
    +        SolrException solrException = new SolrException(BAD_REQUEST, 
"Invalid timezone:" + tz);
    +        log.error("Could not create routed alias",solrException);
    +        throw solrException;
    +
    +      }
    +      TimeZone zone;
    +      if (tz != null) {
    +        zone = TimeZoneUtils.getTimeZone(tz);
    +      } else {
    +        zone = TimeZoneUtils.getTimeZone("UTC");
    +      }
    +      DateTimeFormatter fmt = 
DATE_TIME_FORMATTER.withZone(zone.toZoneId());
    --- End diff --
    
    In reading the issue referenced in the above referenced comment again, it 
occurs to me that what I hadn't thought about is that by having TZ parameter at 
the top level it does sort of imply that it applies to the whole request, and 
we need to handle all dates supplied either as the specified UTC or if the Z is 
omitted handle them as the timezone specified. I had mostly been thinking in 
terms of the effect of TZ on updates and users trying to optimize by querying 
directly to the correct collection and the calculation of "next" collections. 
In theory, there could be 2 TZ's one for the routing configuration router.tz 
and one that applies to the dates in this request (the top level TZ)... but 
that is probably be overkill. So now I think I understand what you've been 
trying to tell me... you want TZ to adjust any dates that don't include a 
timezone, and not adjust any that do (where the only acceptable date included 
TZ is utc via the Z suffix)?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to