Details

    • Type: New Feature
    • Status: Closed
    • Priority: Major
    • Resolution: Resolved
    • Affects Version/s: None
    • Fix Version/s: 6.6, 7.0
    • Component/s: None
    • Security Level: Public (Default Security Level. Issues are Public)
    • Labels:
      None

      Description

      This ticket will add Stream Evaluators that extract date/time values from a Solr date field. The following Evaluators will be supported:

      hour (date)
      minute (date)
      month (date)
      monthname(date)
      quarter(date)
      second (date)
      year(date)

      Syntax:

      select(id,
             year(recdate) as year,
             month(recdate) as month,
             day(recdate) as day,
             search(logs, q="blah", fl="id, recdate", sort="recdate asc", qt="/export"))
      

        Issue Links

          Activity

          Hide
          covolution Gethin James added a comment -

          I started working on this. With support for:

          year, month, day, dayofyear, dayofquarter, hour, minute, quarter, week, second, epoch

          Initially supporting a String field but I am hoping to add Date, Instant support.
          https://github.com/covolution/lucene-solr/tree/SOLR-10303

          Show
          covolution Gethin James added a comment - I started working on this. With support for: year, month, day, dayofyear, dayofquarter, hour, minute, quarter, week, second, epoch Initially supporting a String field but I am hoping to add Date, Instant support. https://github.com/covolution/lucene-solr/tree/SOLR-10303
          Hide
          dpgove Dennis Gove added a comment -

          Can we call this a DatePartEvaluator?

          This seems to only work on field values (via the field name). There'll need to be support for acting on the results of other evaluators.

          Show
          dpgove Dennis Gove added a comment - Can we call this a DatePartEvaluator? This seems to only work on field values (via the field name). There'll need to be support for acting on the results of other evaluators.
          Hide
          covolution Gethin James added a comment -

          I renamed the evaluator as suggested and added support for Instant, Date, LocalDateTime.

          I was wondering if there's an advantage to extending NumberEvaluator rather than SimpleEvaluator? The code currently extends NumberEvaluator, hence only returns number, but this ticket mentions a "monthname" function; that suggests a need for a Locale and a return type of String. Possibly that's a different function?

          Show
          covolution Gethin James added a comment - I renamed the evaluator as suggested and added support for Instant, Date, LocalDateTime. I was wondering if there's an advantage to extending NumberEvaluator rather than SimpleEvaluator? The code currently extends NumberEvaluator, hence only returns number, but this ticket mentions a "monthname" function; that suggests a need for a Locale and a return type of String. Possibly that's a different function?
          Hide
          covolution Gethin James added a comment -

          Adding a patch file or would you prefer a pull request?

          Show
          covolution Gethin James added a comment - Adding a patch file or would you prefer a pull request?
          Hide
          dpgove Dennis Gove added a comment -

          For the function names, any concern with using pascal casing for dayOfYear, dayOfMonth? That will be consistent with other multi-word cases in streaming.

          Show
          dpgove Dennis Gove added a comment - For the function names, any concern with using pascal casing for dayOfYear, dayOfMonth? That will be consistent with other multi-word cases in streaming.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user covolution opened a pull request:

          https://github.com/apache/lucene-solr/pull/171

          SOLR-10303

          Adding date/time Stream Evaluators for year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/covolution/lucene-solr SOLR-10303

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/lucene-solr/pull/171.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #171



          Show
          githubbot ASF GitHub Bot added a comment - GitHub user covolution opened a pull request: https://github.com/apache/lucene-solr/pull/171 SOLR-10303 Adding date/time Stream Evaluators for year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch. You can merge this pull request into a Git repository by running: $ git pull https://github.com/covolution/lucene-solr SOLR-10303 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/171.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #171
          Hide
          covolution Gethin James added a comment -

          We could do with some documentation! Here is a start:

          Date functions
          Date Stream Evaluators can be used to evaluate the date part of other field values in a tuple. Supported data types are String in the ISO_INSTANT date format, Instant, java.util.Date, LocalDateTime or an instance of TemporalAccessor.

          eg.

          year(fieldA)  //where fieldA is a field with a String in the ISO_INSTANT date format
          month(fieldA)
          hour(fieldA)
          
          • year - The year part of a date
          • month - The month of the year
          • day - The day of the month
          • dayOfYear - The day of the year
          • dayOfQuarter - The day of the quarter
          • hour - hour of the day, from 0 to 23.
          • minute - minute of the hour
          • quarter - quarter of the year
          • week - Iso standard week of a year, 1 to 53 weeks.
          • second - second of the minute
          • epoch - number of milliseconds from the epoch
          Show
          covolution Gethin James added a comment - We could do with some documentation! Here is a start: Date functions Date Stream Evaluators can be used to evaluate the date part of other field values in a tuple. Supported data types are String in the ISO_INSTANT date format, Instant, java.util.Date, LocalDateTime or an instance of TemporalAccessor. eg. year(fieldA) //where fieldA is a field with a String in the ISO_INSTANT date format month(fieldA) hour(fieldA) year - The year part of a date month - The month of the year day - The day of the month dayOfYear - The day of the year dayOfQuarter - The day of the quarter hour - hour of the day, from 0 to 23. minute - minute of the hour quarter - quarter of the year week - Iso standard week of a year, 1 to 53 weeks. second - second of the minute epoch - number of milliseconds from the epoch
          Hide
          joel.bernstein Joel Bernstein added a comment - - edited

          I really like the design of this. A couple things to think about:

          1) We could create the concept of a TupleContext, which the SelectStream passes to each evaluator. This will allow us to share LocalDateTime instances across date/time evaluators. The logic would be: check to see if the LocalDateTime for a specific field is already in the TupleContext, and use it rather then parsing a new Instant. The SelectStream would clear the TupleContext after each Tuple is read. This will speed up selects that call multiple date evaluators on the same field:

          select(expr,
                    year(field1) as year,
                    month(field1) as month,
                    day(field1) as day)
          

          Alternatively we could create a TimeContext which would only be passed to DateTimeEvalutors, which wouldn't mean changing all the Evaluators to accept a TupleContext.

          2) I think we only need to support two formats to parse from: the date string format and the Long epoch time.

          Show
          joel.bernstein Joel Bernstein added a comment - - edited I really like the design of this. A couple things to think about: 1) We could create the concept of a TupleContext, which the SelectStream passes to each evaluator. This will allow us to share LocalDateTime instances across date/time evaluators. The logic would be: check to see if the LocalDateTime for a specific field is already in the TupleContext, and use it rather then parsing a new Instant. The SelectStream would clear the TupleContext after each Tuple is read. This will speed up selects that call multiple date evaluators on the same field: select(expr, year(field1) as year, month(field1) as month, day(field1) as day) Alternatively we could create a TimeContext which would only be passed to DateTimeEvalutors, which wouldn't mean changing all the Evaluators to accept a TupleContext. 2) I think we only need to support two formats to parse from: the date string format and the Long epoch time.
          Hide
          covolution Gethin James added a comment -

          1) I like the idea of a TupleContext which the SelectStream can pass along. Are you thinking that should be done as part of this ticket? Currently the implementation just acts on tuple evaluations so there is nothing specific for SelectStream.
          2) The current implementation support a variety of types: String, Instant, Date, TemporalAccessor. Its missing long support so I can add that.

          Show
          covolution Gethin James added a comment - 1) I like the idea of a TupleContext which the SelectStream can pass along. Are you thinking that should be done as part of this ticket? Currently the implementation just acts on tuple evaluations so there is nothing specific for SelectStream. 2) The current implementation support a variety of types: String, Instant, Date, TemporalAccessor. Its missing long support so I can add that.
          Hide
          dpgove Dennis Gove added a comment -

          In case you missed them, I added comments to the PR at https://github.com/apache/lucene-solr/pull/171

          Show
          dpgove Dennis Gove added a comment - In case you missed them, I added comments to the PR at https://github.com/apache/lucene-solr/pull/171
          Hide
          covolution Gethin James added a comment -

          Sorry, I can't find any comments on the pull request.

          Show
          covolution Gethin James added a comment - Sorry, I can't find any comments on the pull request.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r107802207

          — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java —
          @@ -199,10 +200,16 @@ public void inform(SolrCore core) {
          .withFunctionName("mult", MultiplyEvaluator.class)
          .withFunctionName("sub", SubtractEvaluator.class)
          .withFunctionName("log", NaturalLogEvaluator.class)
          +
          // Conditional Stream Evaluators
          .withFunctionName("if", IfThenElseEvaluator.class)
          ;

          + // Date evaluators
          — End diff –

          I'm not a huge fan of using the same class to handle multiple functions. There are places where we use the class to find the function name and if > 1 functions are mapped to a class then these lookups no longer work.

          See [this](https://github.com/dennisgove/lucene-solr/blob/master/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamFactory.java#L397) for where it wouldn't work.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dennisgove commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r107802207 — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java — @@ -199,10 +200,16 @@ public void inform(SolrCore core) { .withFunctionName("mult", MultiplyEvaluator.class) .withFunctionName("sub", SubtractEvaluator.class) .withFunctionName("log", NaturalLogEvaluator.class) + // Conditional Stream Evaluators .withFunctionName("if", IfThenElseEvaluator.class) ; + // Date evaluators — End diff – I'm not a huge fan of using the same class to handle multiple functions. There are places where we use the class to find the function name and if > 1 functions are mapped to a class then these lookups no longer work. See [this] ( https://github.com/dennisgove/lucene-solr/blob/master/solr/solrj/src/java/org/apache/solr/client/solrj/io/stream/expr/StreamFactory.java#L397 ) for where it wouldn't work.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r107804517

          — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java —
          @@ -0,0 +1,169 @@
          +/*
          + * 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.solr.client.solrj.io.eval;
          +
          +import java.io.IOException;
          +import java.time.Instant;
          +import java.time.LocalDateTime;
          +import java.time.ZoneOffset;
          +import java.time.format.DateTimeParseException;
          +import java.time.temporal.ChronoField;
          +import java.time.temporal.IsoFields;
          +import java.time.temporal.TemporalAccessor;
          +import java.time.temporal.UnsupportedTemporalTypeException;
          +import java.util.Arrays;
          +import java.util.Date;
          +import java.util.Locale;
          +
          +import org.apache.solr.client.solrj.io.Tuple;
          +import org.apache.solr.client.solrj.io.stream.expr.Explanation;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory;
          +
          +/**
          + * Provides numeric Date/Time stream evaluators
          + */
          +public class DatePartEvaluator extends NumberEvaluator {
          +
          + public enum FUNCTION

          {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch}

          +
          + private final FUNCTION function;
          +
          + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException {
          + super(expression, factory);
          +
          + String functionName = expression.getFunctionName();
          +
          + try

          { + this.function = FUNCTION.valueOf(functionName); + }

          catch (IllegalArgumentException e)

          { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + }

          +
          + if (1 != subEvaluators.size())

          { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + }

          + }
          +
          + @Override
          + public Number evaluate(Tuple tuple) throws IOException {
          +
          + Instant instant = null;
          + TemporalAccessor date = null;
          +
          + //First evaluate the parameter
          + StreamEvaluator streamEvaluator = subEvaluators.get(0);
          + Object tupleValue = streamEvaluator.evaluate(tuple);
          +
          + if (tupleValue == null) return null;
          +
          + if (tupleValue instanceof String)

          { + instant = getInstant((String) tupleValue); + }

          else if (tupleValue instanceof Instant)

          { + instant = (Instant) tupleValue; + }

          else if (tupleValue instanceof Date)

          { + instant = ((Date) tupleValue).toInstant(); + }

          else if (tupleValue instanceof TemporalAccessor)

          { + date = ((TemporalAccessor) tupleValue); + }

          +
          + if (instant != null) {
          + if (function.equals(FUNCTION.epoch)) return instant.toEpochMilli();
          — End diff –

          Even in a case like this, why not pass the instant down to the `private evaluate(Instant)` function. That way all the real decisions are made there.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dennisgove commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r107804517 — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java — @@ -0,0 +1,169 @@ +/* + * 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.solr.client.solrj.io.eval; + +import java.io.IOException; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.format.DateTimeParseException; +import java.time.temporal.ChronoField; +import java.time.temporal.IsoFields; +import java.time.temporal.TemporalAccessor; +import java.time.temporal.UnsupportedTemporalTypeException; +import java.util.Arrays; +import java.util.Date; +import java.util.Locale; + +import org.apache.solr.client.solrj.io.Tuple; +import org.apache.solr.client.solrj.io.stream.expr.Explanation; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter; +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory; + +/** + * Provides numeric Date/Time stream evaluators + */ +public class DatePartEvaluator extends NumberEvaluator { + + public enum FUNCTION {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch} + + private final FUNCTION function; + + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException { + super(expression, factory); + + String functionName = expression.getFunctionName(); + + try { + this.function = FUNCTION.valueOf(functionName); + } catch (IllegalArgumentException e) { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + } + + if (1 != subEvaluators.size()) { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + } + } + + @Override + public Number evaluate(Tuple tuple) throws IOException { + + Instant instant = null; + TemporalAccessor date = null; + + //First evaluate the parameter + StreamEvaluator streamEvaluator = subEvaluators.get(0); + Object tupleValue = streamEvaluator.evaluate(tuple); + + if (tupleValue == null) return null; + + if (tupleValue instanceof String) { + instant = getInstant((String) tupleValue); + } else if (tupleValue instanceof Instant) { + instant = (Instant) tupleValue; + } else if (tupleValue instanceof Date) { + instant = ((Date) tupleValue).toInstant(); + } else if (tupleValue instanceof TemporalAccessor) { + date = ((TemporalAccessor) tupleValue); + } + + if (instant != null) { + if (function.equals(FUNCTION.epoch)) return instant.toEpochMilli(); — End diff – Even in a case like this, why not pass the instant down to the `private evaluate(Instant)` function. That way all the real decisions are made there.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r107804393

          — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java —
          @@ -0,0 +1,169 @@
          +/*
          + * 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.solr.client.solrj.io.eval;
          +
          +import java.io.IOException;
          +import java.time.Instant;
          +import java.time.LocalDateTime;
          +import java.time.ZoneOffset;
          +import java.time.format.DateTimeParseException;
          +import java.time.temporal.ChronoField;
          +import java.time.temporal.IsoFields;
          +import java.time.temporal.TemporalAccessor;
          +import java.time.temporal.UnsupportedTemporalTypeException;
          +import java.util.Arrays;
          +import java.util.Date;
          +import java.util.Locale;
          +
          +import org.apache.solr.client.solrj.io.Tuple;
          +import org.apache.solr.client.solrj.io.stream.expr.Explanation;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory;
          +
          +/**
          + * Provides numeric Date/Time stream evaluators
          + */
          +public class DatePartEvaluator extends NumberEvaluator {
          +
          + public enum FUNCTION

          {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch}

          +
          + private final FUNCTION function;
          +
          + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException {
          + super(expression, factory);
          +
          + String functionName = expression.getFunctionName();
          +
          + try

          { + this.function = FUNCTION.valueOf(functionName); + }

          catch (IllegalArgumentException e)

          { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + }

          +
          + if (1 != subEvaluators.size())

          { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + }

          + }
          +
          + @Override
          + public Number evaluate(Tuple tuple) throws IOException {
          +
          + Instant instant = null;
          + TemporalAccessor date = null;
          +
          + //First evaluate the parameter
          + StreamEvaluator streamEvaluator = subEvaluators.get(0);
          + Object tupleValue = streamEvaluator.evaluate(tuple);
          +
          + if (tupleValue == null) return null;
          +
          + if (tupleValue instanceof String)

          { + instant = getInstant((String) tupleValue); + }

          else if (tupleValue instanceof Instant)

          { + instant = (Instant) tupleValue; + }

          else if (tupleValue instanceof Date)

          { + instant = ((Date) tupleValue).toInstant(); + }

          else if (tupleValue instanceof TemporalAccessor)

          { + date = ((TemporalAccessor) tupleValue); + }

          +
          + if (instant != null) {
          — End diff –

          I'd rather see this continue to act on an Instant instead of a Date. If the value is a TemporalAccessor (above if statement) you can convert that into an Instant.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dennisgove commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r107804393 — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java — @@ -0,0 +1,169 @@ +/* + * 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.solr.client.solrj.io.eval; + +import java.io.IOException; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.format.DateTimeParseException; +import java.time.temporal.ChronoField; +import java.time.temporal.IsoFields; +import java.time.temporal.TemporalAccessor; +import java.time.temporal.UnsupportedTemporalTypeException; +import java.util.Arrays; +import java.util.Date; +import java.util.Locale; + +import org.apache.solr.client.solrj.io.Tuple; +import org.apache.solr.client.solrj.io.stream.expr.Explanation; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter; +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory; + +/** + * Provides numeric Date/Time stream evaluators + */ +public class DatePartEvaluator extends NumberEvaluator { + + public enum FUNCTION {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch} + + private final FUNCTION function; + + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException { + super(expression, factory); + + String functionName = expression.getFunctionName(); + + try { + this.function = FUNCTION.valueOf(functionName); + } catch (IllegalArgumentException e) { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + } + + if (1 != subEvaluators.size()) { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + } + } + + @Override + public Number evaluate(Tuple tuple) throws IOException { + + Instant instant = null; + TemporalAccessor date = null; + + //First evaluate the parameter + StreamEvaluator streamEvaluator = subEvaluators.get(0); + Object tupleValue = streamEvaluator.evaluate(tuple); + + if (tupleValue == null) return null; + + if (tupleValue instanceof String) { + instant = getInstant((String) tupleValue); + } else if (tupleValue instanceof Instant) { + instant = (Instant) tupleValue; + } else if (tupleValue instanceof Date) { + instant = ((Date) tupleValue).toInstant(); + } else if (tupleValue instanceof TemporalAccessor) { + date = ((TemporalAccessor) tupleValue); + } + + if (instant != null) { — End diff – I'd rather see this continue to act on an Instant instead of a Date. If the value is a TemporalAccessor (above if statement) you can convert that into an Instant.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r107805373

          — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java —
          @@ -0,0 +1,169 @@
          +/*
          + * 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.solr.client.solrj.io.eval;
          +
          +import java.io.IOException;
          +import java.time.Instant;
          +import java.time.LocalDateTime;
          +import java.time.ZoneOffset;
          +import java.time.format.DateTimeParseException;
          +import java.time.temporal.ChronoField;
          +import java.time.temporal.IsoFields;
          +import java.time.temporal.TemporalAccessor;
          +import java.time.temporal.UnsupportedTemporalTypeException;
          +import java.util.Arrays;
          +import java.util.Date;
          +import java.util.Locale;
          +
          +import org.apache.solr.client.solrj.io.Tuple;
          +import org.apache.solr.client.solrj.io.stream.expr.Explanation;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory;
          +
          +/**
          + * Provides numeric Date/Time stream evaluators
          + */
          +public class DatePartEvaluator extends NumberEvaluator {
          +
          + public enum FUNCTION

          {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch}

          +
          + private final FUNCTION function;
          +
          + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException {
          + super(expression, factory);
          +
          + String functionName = expression.getFunctionName();
          +
          + try

          { + this.function = FUNCTION.valueOf(functionName); + }

          catch (IllegalArgumentException e)

          { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + }

          +
          + if (1 != subEvaluators.size())

          { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + }

          + }
          +
          + @Override
          + public Number evaluate(Tuple tuple) throws IOException {
          +
          + Instant instant = null;
          + TemporalAccessor date = null;
          +
          + //First evaluate the parameter
          + StreamEvaluator streamEvaluator = subEvaluators.get(0);
          + Object tupleValue = streamEvaluator.evaluate(tuple);
          +
          + if (tupleValue == null) return null;
          +
          + if (tupleValue instanceof String)

          { + instant = getInstant((String) tupleValue); + }

          else if (tupleValue instanceof Instant)

          { + instant = (Instant) tupleValue; + }

          else if (tupleValue instanceof Date)

          { + instant = ((Date) tupleValue).toInstant(); + }

          else if (tupleValue instanceof TemporalAccessor)

          { + date = ((TemporalAccessor) tupleValue); + }

          +
          + if (instant != null)

          { + if (function.equals(FUNCTION.epoch)) return instant.toEpochMilli(); + date = LocalDateTime.ofInstant(instant, ZoneOffset.UTC); + }

          +
          + if (date != null)

          { + return evaluate(date); + }

          +
          + throw new IOException(String.format(Locale.ROOT, "Invalid parameter %s - The parameter must be a string formatted ISO_INSTANT or of type Instant,Date or LocalDateTime.", String.valueOf(tupleValue)));
          + }
          +
          + private Instant getInstant(String dateStr) throws IOException {
          +
          + if (dateStr != null && !dateStr.isEmpty()) {
          + try

          { + return Instant.parse(dateStr); + }

          catch (DateTimeParseException e)

          { + throw new IOException(String.format(Locale.ROOT, "Invalid parameter %s - The String must be formatted in the ISO_INSTANT date format.", dateStr)); + }

          + }
          + return null;
          + }
          +
          + /**
          + * Evaluate the date based on the specified function
          + *
          + * @param date
          + * @return the evaluated value
          + */
          + private Number evaluate(TemporalAccessor date) throws IOException {
          — End diff –

          If you were to break this class into multiple (one for each evaluator) then this function should def exist in a parent class so keep the logic in a singular place.

          The way I could see the hierarchy working is something like this
          ```
          ComplexEvaluator
          TemporalEvaluator
          YearEvaluator
          MonthOfYearEvaluator
          DayOfMonthEvalator
          DayNameEvaluator
          ....
          ```

          Basically, if some evaluator extends NumberEvaluator then that means this evaluator works over numbers, not that it returns a number. In a sense, when the sub-evaluators are executed they need to return something that is a Number. Following that logic, these evaluators work over temporal objects, so they extend TemporalEvaluator.

          They can return whatever type they need to (String, Number, etc...) but they must have a temporal value to work from.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dennisgove commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r107805373 — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java — @@ -0,0 +1,169 @@ +/* + * 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.solr.client.solrj.io.eval; + +import java.io.IOException; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.format.DateTimeParseException; +import java.time.temporal.ChronoField; +import java.time.temporal.IsoFields; +import java.time.temporal.TemporalAccessor; +import java.time.temporal.UnsupportedTemporalTypeException; +import java.util.Arrays; +import java.util.Date; +import java.util.Locale; + +import org.apache.solr.client.solrj.io.Tuple; +import org.apache.solr.client.solrj.io.stream.expr.Explanation; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter; +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory; + +/** + * Provides numeric Date/Time stream evaluators + */ +public class DatePartEvaluator extends NumberEvaluator { + + public enum FUNCTION {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch} + + private final FUNCTION function; + + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException { + super(expression, factory); + + String functionName = expression.getFunctionName(); + + try { + this.function = FUNCTION.valueOf(functionName); + } catch (IllegalArgumentException e) { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + } + + if (1 != subEvaluators.size()) { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + } + } + + @Override + public Number evaluate(Tuple tuple) throws IOException { + + Instant instant = null; + TemporalAccessor date = null; + + //First evaluate the parameter + StreamEvaluator streamEvaluator = subEvaluators.get(0); + Object tupleValue = streamEvaluator.evaluate(tuple); + + if (tupleValue == null) return null; + + if (tupleValue instanceof String) { + instant = getInstant((String) tupleValue); + } else if (tupleValue instanceof Instant) { + instant = (Instant) tupleValue; + } else if (tupleValue instanceof Date) { + instant = ((Date) tupleValue).toInstant(); + } else if (tupleValue instanceof TemporalAccessor) { + date = ((TemporalAccessor) tupleValue); + } + + if (instant != null) { + if (function.equals(FUNCTION.epoch)) return instant.toEpochMilli(); + date = LocalDateTime.ofInstant(instant, ZoneOffset.UTC); + } + + if (date != null) { + return evaluate(date); + } + + throw new IOException(String.format(Locale.ROOT, "Invalid parameter %s - The parameter must be a string formatted ISO_INSTANT or of type Instant,Date or LocalDateTime.", String.valueOf(tupleValue))); + } + + private Instant getInstant(String dateStr) throws IOException { + + if (dateStr != null && !dateStr.isEmpty()) { + try { + return Instant.parse(dateStr); + } catch (DateTimeParseException e) { + throw new IOException(String.format(Locale.ROOT, "Invalid parameter %s - The String must be formatted in the ISO_INSTANT date format.", dateStr)); + } + } + return null; + } + + /** + * Evaluate the date based on the specified function + * + * @param date + * @return the evaluated value + */ + private Number evaluate(TemporalAccessor date) throws IOException { — End diff – If you were to break this class into multiple (one for each evaluator) then this function should def exist in a parent class so keep the logic in a singular place. The way I could see the hierarchy working is something like this ``` ComplexEvaluator TemporalEvaluator YearEvaluator MonthOfYearEvaluator DayOfMonthEvalator DayNameEvaluator .... ``` Basically, if some evaluator extends NumberEvaluator then that means this evaluator works over numbers, not that it returns a number. In a sense, when the sub-evaluators are executed they need to return something that is a Number. Following that logic, these evaluators work over temporal objects, so they extend TemporalEvaluator. They can return whatever type they need to (String, Number, etc...) but they must have a temporal value to work from.
          Hide
          dpgove Dennis Gove added a comment -

          Gethin James I'm sorry. I forgot to submit the review on Github.

          Show
          dpgove Dennis Gove added a comment - Gethin James I'm sorry. I forgot to submit the review on Github.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r110107074

          — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java —
          @@ -199,10 +200,16 @@ public void inform(SolrCore core) {
          .withFunctionName("mult", MultiplyEvaluator.class)
          .withFunctionName("sub", SubtractEvaluator.class)
          .withFunctionName("log", NaturalLogEvaluator.class)
          +
          // Conditional Stream Evaluators
          .withFunctionName("if", IfThenElseEvaluator.class)
          ;

          + // Date evaluators
          — End diff –

          I figured using one class (< 170 lines) to implement 11 date functions was preferable to lots of little classes. Now I know there's an assumption that 1 class = 1 function, I can re-factor to add all the extra classes.

          Show
          githubbot ASF GitHub Bot added a comment - Github user covolution commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r110107074 — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java — @@ -199,10 +200,16 @@ public void inform(SolrCore core) { .withFunctionName("mult", MultiplyEvaluator.class) .withFunctionName("sub", SubtractEvaluator.class) .withFunctionName("log", NaturalLogEvaluator.class) + // Conditional Stream Evaluators .withFunctionName("if", IfThenElseEvaluator.class) ; + // Date evaluators — End diff – I figured using one class (< 170 lines) to implement 11 date functions was preferable to lots of little classes. Now I know there's an assumption that 1 class = 1 function, I can re-factor to add all the extra classes.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r110107181

          — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java —
          @@ -0,0 +1,169 @@
          +/*
          + * 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.solr.client.solrj.io.eval;
          +
          +import java.io.IOException;
          +import java.time.Instant;
          +import java.time.LocalDateTime;
          +import java.time.ZoneOffset;
          +import java.time.format.DateTimeParseException;
          +import java.time.temporal.ChronoField;
          +import java.time.temporal.IsoFields;
          +import java.time.temporal.TemporalAccessor;
          +import java.time.temporal.UnsupportedTemporalTypeException;
          +import java.util.Arrays;
          +import java.util.Date;
          +import java.util.Locale;
          +
          +import org.apache.solr.client.solrj.io.Tuple;
          +import org.apache.solr.client.solrj.io.stream.expr.Explanation;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory;
          +
          +/**
          + * Provides numeric Date/Time stream evaluators
          + */
          +public class DatePartEvaluator extends NumberEvaluator {
          +
          + public enum FUNCTION

          {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch}

          +
          + private final FUNCTION function;
          +
          + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException {
          + super(expression, factory);
          +
          + String functionName = expression.getFunctionName();
          +
          + try

          { + this.function = FUNCTION.valueOf(functionName); + }

          catch (IllegalArgumentException e)

          { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + }

          +
          + if (1 != subEvaluators.size())

          { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + }

          + }
          +
          + @Override
          + public Number evaluate(Tuple tuple) throws IOException {
          +
          + Instant instant = null;
          + TemporalAccessor date = null;
          +
          + //First evaluate the parameter
          + StreamEvaluator streamEvaluator = subEvaluators.get(0);
          + Object tupleValue = streamEvaluator.evaluate(tuple);
          +
          + if (tupleValue == null) return null;
          +
          + if (tupleValue instanceof String)

          { + instant = getInstant((String) tupleValue); + }

          else if (tupleValue instanceof Instant)

          { + instant = (Instant) tupleValue; + }

          else if (tupleValue instanceof Date)

          { + instant = ((Date) tupleValue).toInstant(); + }

          else if (tupleValue instanceof TemporalAccessor)

          { + date = ((TemporalAccessor) tupleValue); + }

          +
          + if (instant != null) {
          — End diff –

          Instant does not work with human units of time(eg. year, month, or day), a timezone is required. So I am converting it to a LocalDateTime using ZoneOffset.UTC.

          Show
          githubbot ASF GitHub Bot added a comment - Github user covolution commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r110107181 — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java — @@ -0,0 +1,169 @@ +/* + * 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.solr.client.solrj.io.eval; + +import java.io.IOException; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.format.DateTimeParseException; +import java.time.temporal.ChronoField; +import java.time.temporal.IsoFields; +import java.time.temporal.TemporalAccessor; +import java.time.temporal.UnsupportedTemporalTypeException; +import java.util.Arrays; +import java.util.Date; +import java.util.Locale; + +import org.apache.solr.client.solrj.io.Tuple; +import org.apache.solr.client.solrj.io.stream.expr.Explanation; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter; +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory; + +/** + * Provides numeric Date/Time stream evaluators + */ +public class DatePartEvaluator extends NumberEvaluator { + + public enum FUNCTION {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch} + + private final FUNCTION function; + + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException { + super(expression, factory); + + String functionName = expression.getFunctionName(); + + try { + this.function = FUNCTION.valueOf(functionName); + } catch (IllegalArgumentException e) { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + } + + if (1 != subEvaluators.size()) { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + } + } + + @Override + public Number evaluate(Tuple tuple) throws IOException { + + Instant instant = null; + TemporalAccessor date = null; + + //First evaluate the parameter + StreamEvaluator streamEvaluator = subEvaluators.get(0); + Object tupleValue = streamEvaluator.evaluate(tuple); + + if (tupleValue == null) return null; + + if (tupleValue instanceof String) { + instant = getInstant((String) tupleValue); + } else if (tupleValue instanceof Instant) { + instant = (Instant) tupleValue; + } else if (tupleValue instanceof Date) { + instant = ((Date) tupleValue).toInstant(); + } else if (tupleValue instanceof TemporalAccessor) { + date = ((TemporalAccessor) tupleValue); + } + + if (instant != null) { — End diff – Instant does not work with human units of time(eg. year, month, or day), a timezone is required. So I am converting it to a LocalDateTime using ZoneOffset.UTC.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r110107246

          — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java —
          @@ -0,0 +1,169 @@
          +/*
          + * 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.solr.client.solrj.io.eval;
          +
          +import java.io.IOException;
          +import java.time.Instant;
          +import java.time.LocalDateTime;
          +import java.time.ZoneOffset;
          +import java.time.format.DateTimeParseException;
          +import java.time.temporal.ChronoField;
          +import java.time.temporal.IsoFields;
          +import java.time.temporal.TemporalAccessor;
          +import java.time.temporal.UnsupportedTemporalTypeException;
          +import java.util.Arrays;
          +import java.util.Date;
          +import java.util.Locale;
          +
          +import org.apache.solr.client.solrj.io.Tuple;
          +import org.apache.solr.client.solrj.io.stream.expr.Explanation;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory;
          +
          +/**
          + * Provides numeric Date/Time stream evaluators
          + */
          +public class DatePartEvaluator extends NumberEvaluator {
          +
          + public enum FUNCTION

          {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch}

          +
          + private final FUNCTION function;
          +
          + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException {
          + super(expression, factory);
          +
          + String functionName = expression.getFunctionName();
          +
          + try

          { + this.function = FUNCTION.valueOf(functionName); + }

          catch (IllegalArgumentException e)

          { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + }

          +
          + if (1 != subEvaluators.size())

          { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + }

          + }
          +
          + @Override
          + public Number evaluate(Tuple tuple) throws IOException {
          +
          + Instant instant = null;
          + TemporalAccessor date = null;
          +
          + //First evaluate the parameter
          + StreamEvaluator streamEvaluator = subEvaluators.get(0);
          + Object tupleValue = streamEvaluator.evaluate(tuple);
          +
          + if (tupleValue == null) return null;
          +
          + if (tupleValue instanceof String)

          { + instant = getInstant((String) tupleValue); + }

          else if (tupleValue instanceof Instant)

          { + instant = (Instant) tupleValue; + }

          else if (tupleValue instanceof Date)

          { + instant = ((Date) tupleValue).toInstant(); + }

          else if (tupleValue instanceof TemporalAccessor)

          { + date = ((TemporalAccessor) tupleValue); + }

          +
          + if (instant != null) {
          + if (function.equals(FUNCTION.epoch)) return instant.toEpochMilli();
          — End diff –

          This is an optimization to avoid creating a Date object unnecessarily.

          Show
          githubbot ASF GitHub Bot added a comment - Github user covolution commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r110107246 — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java — @@ -0,0 +1,169 @@ +/* + * 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.solr.client.solrj.io.eval; + +import java.io.IOException; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.format.DateTimeParseException; +import java.time.temporal.ChronoField; +import java.time.temporal.IsoFields; +import java.time.temporal.TemporalAccessor; +import java.time.temporal.UnsupportedTemporalTypeException; +import java.util.Arrays; +import java.util.Date; +import java.util.Locale; + +import org.apache.solr.client.solrj.io.Tuple; +import org.apache.solr.client.solrj.io.stream.expr.Explanation; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter; +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory; + +/** + * Provides numeric Date/Time stream evaluators + */ +public class DatePartEvaluator extends NumberEvaluator { + + public enum FUNCTION {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch} + + private final FUNCTION function; + + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException { + super(expression, factory); + + String functionName = expression.getFunctionName(); + + try { + this.function = FUNCTION.valueOf(functionName); + } catch (IllegalArgumentException e) { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + } + + if (1 != subEvaluators.size()) { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + } + } + + @Override + public Number evaluate(Tuple tuple) throws IOException { + + Instant instant = null; + TemporalAccessor date = null; + + //First evaluate the parameter + StreamEvaluator streamEvaluator = subEvaluators.get(0); + Object tupleValue = streamEvaluator.evaluate(tuple); + + if (tupleValue == null) return null; + + if (tupleValue instanceof String) { + instant = getInstant((String) tupleValue); + } else if (tupleValue instanceof Instant) { + instant = (Instant) tupleValue; + } else if (tupleValue instanceof Date) { + instant = ((Date) tupleValue).toInstant(); + } else if (tupleValue instanceof TemporalAccessor) { + date = ((TemporalAccessor) tupleValue); + } + + if (instant != null) { + if (function.equals(FUNCTION.epoch)) return instant.toEpochMilli(); — End diff – This is an optimization to avoid creating a Date object unnecessarily.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r110107409

          — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java —
          @@ -0,0 +1,169 @@
          +/*
          + * 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.solr.client.solrj.io.eval;
          +
          +import java.io.IOException;
          +import java.time.Instant;
          +import java.time.LocalDateTime;
          +import java.time.ZoneOffset;
          +import java.time.format.DateTimeParseException;
          +import java.time.temporal.ChronoField;
          +import java.time.temporal.IsoFields;
          +import java.time.temporal.TemporalAccessor;
          +import java.time.temporal.UnsupportedTemporalTypeException;
          +import java.util.Arrays;
          +import java.util.Date;
          +import java.util.Locale;
          +
          +import org.apache.solr.client.solrj.io.Tuple;
          +import org.apache.solr.client.solrj.io.stream.expr.Explanation;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter;
          +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory;
          +
          +/**
          + * Provides numeric Date/Time stream evaluators
          + */
          +public class DatePartEvaluator extends NumberEvaluator {
          +
          + public enum FUNCTION

          {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch}

          +
          + private final FUNCTION function;
          +
          + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException {
          + super(expression, factory);
          +
          + String functionName = expression.getFunctionName();
          +
          + try

          { + this.function = FUNCTION.valueOf(functionName); + }

          catch (IllegalArgumentException e)

          { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + }

          +
          + if (1 != subEvaluators.size())

          { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + }

          + }
          +
          + @Override
          + public Number evaluate(Tuple tuple) throws IOException {
          +
          + Instant instant = null;
          + TemporalAccessor date = null;
          +
          + //First evaluate the parameter
          + StreamEvaluator streamEvaluator = subEvaluators.get(0);
          + Object tupleValue = streamEvaluator.evaluate(tuple);
          +
          + if (tupleValue == null) return null;
          +
          + if (tupleValue instanceof String)

          { + instant = getInstant((String) tupleValue); + }

          else if (tupleValue instanceof Instant)

          { + instant = (Instant) tupleValue; + }

          else if (tupleValue instanceof Date)

          { + instant = ((Date) tupleValue).toInstant(); + }

          else if (tupleValue instanceof TemporalAccessor)

          { + date = ((TemporalAccessor) tupleValue); + }

          +
          + if (instant != null)

          { + if (function.equals(FUNCTION.epoch)) return instant.toEpochMilli(); + date = LocalDateTime.ofInstant(instant, ZoneOffset.UTC); + }

          +
          + if (date != null)

          { + return evaluate(date); + }

          +
          + throw new IOException(String.format(Locale.ROOT, "Invalid parameter %s - The parameter must be a string formatted ISO_INSTANT or of type Instant,Date or LocalDateTime.", String.valueOf(tupleValue)));
          + }
          +
          + private Instant getInstant(String dateStr) throws IOException {
          +
          + if (dateStr != null && !dateStr.isEmpty()) {
          + try

          { + return Instant.parse(dateStr); + }

          catch (DateTimeParseException e)

          { + throw new IOException(String.format(Locale.ROOT, "Invalid parameter %s - The String must be formatted in the ISO_INSTANT date format.", dateStr)); + }

          + }
          + return null;
          + }
          +
          + /**
          + * Evaluate the date based on the specified function
          + *
          + * @param date
          + * @return the evaluated value
          + */
          + private Number evaluate(TemporalAccessor date) throws IOException {
          — End diff –

          I will start working on the new design.

          Show
          githubbot ASF GitHub Bot added a comment - Github user covolution commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r110107409 — Diff: solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java — @@ -0,0 +1,169 @@ +/* + * 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.solr.client.solrj.io.eval; + +import java.io.IOException; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.time.format.DateTimeParseException; +import java.time.temporal.ChronoField; +import java.time.temporal.IsoFields; +import java.time.temporal.TemporalAccessor; +import java.time.temporal.UnsupportedTemporalTypeException; +import java.util.Arrays; +import java.util.Date; +import java.util.Locale; + +import org.apache.solr.client.solrj.io.Tuple; +import org.apache.solr.client.solrj.io.stream.expr.Explanation; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpression; +import org.apache.solr.client.solrj.io.stream.expr.StreamExpressionParameter; +import org.apache.solr.client.solrj.io.stream.expr.StreamFactory; + +/** + * Provides numeric Date/Time stream evaluators + */ +public class DatePartEvaluator extends NumberEvaluator { + + public enum FUNCTION {year, month, day, dayOfYear, dayOfQuarter, hour, minute, quarter, week, second, epoch} + + private final FUNCTION function; + + public DatePartEvaluator(StreamExpression expression, StreamFactory factory) throws IOException { + super(expression, factory); + + String functionName = expression.getFunctionName(); + + try { + this.function = FUNCTION.valueOf(functionName); + } catch (IllegalArgumentException e) { + throw new IOException(String.format(Locale.ROOT, "Invalid date expression %s - expecting one of %s", functionName, Arrays.toString(FUNCTION.values()))); + } + + if (1 != subEvaluators.size()) { + throw new IOException(String.format(Locale.ROOT, "Invalid expression %s - expecting one value but found %d", expression, subEvaluators.size())); + } + } + + @Override + public Number evaluate(Tuple tuple) throws IOException { + + Instant instant = null; + TemporalAccessor date = null; + + //First evaluate the parameter + StreamEvaluator streamEvaluator = subEvaluators.get(0); + Object tupleValue = streamEvaluator.evaluate(tuple); + + if (tupleValue == null) return null; + + if (tupleValue instanceof String) { + instant = getInstant((String) tupleValue); + } else if (tupleValue instanceof Instant) { + instant = (Instant) tupleValue; + } else if (tupleValue instanceof Date) { + instant = ((Date) tupleValue).toInstant(); + } else if (tupleValue instanceof TemporalAccessor) { + date = ((TemporalAccessor) tupleValue); + } + + if (instant != null) { + if (function.equals(FUNCTION.epoch)) return instant.toEpochMilli(); + date = LocalDateTime.ofInstant(instant, ZoneOffset.UTC); + } + + if (date != null) { + return evaluate(date); + } + + throw new IOException(String.format(Locale.ROOT, "Invalid parameter %s - The parameter must be a string formatted ISO_INSTANT or of type Instant,Date or LocalDateTime.", String.valueOf(tupleValue))); + } + + private Instant getInstant(String dateStr) throws IOException { + + if (dateStr != null && !dateStr.isEmpty()) { + try { + return Instant.parse(dateStr); + } catch (DateTimeParseException e) { + throw new IOException(String.format(Locale.ROOT, "Invalid parameter %s - The String must be formatted in the ISO_INSTANT date format.", dateStr)); + } + } + return null; + } + + /** + * Evaluate the date based on the specified function + * + * @param date + * @return the evaluated value + */ + private Number evaluate(TemporalAccessor date) throws IOException { — End diff – I will start working on the new design.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r110127292

          — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java —
          @@ -199,10 +200,16 @@ public void inform(SolrCore core) {
          .withFunctionName("mult", MultiplyEvaluator.class)
          .withFunctionName("sub", SubtractEvaluator.class)
          .withFunctionName("log", NaturalLogEvaluator.class)
          +
          // Conditional Stream Evaluators
          .withFunctionName("if", IfThenElseEvaluator.class)
          ;

          + // Date evaluators
          — End diff –

          Yeah, I agree - maybe we should take a look at that assumption of 1 class == 1 function and re-evaluate if there's a better way.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dennisgove commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r110127292 — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java — @@ -199,10 +200,16 @@ public void inform(SolrCore core) { .withFunctionName("mult", MultiplyEvaluator.class) .withFunctionName("sub", SubtractEvaluator.class) .withFunctionName("log", NaturalLogEvaluator.class) + // Conditional Stream Evaluators .withFunctionName("if", IfThenElseEvaluator.class) ; + // Date evaluators — End diff – Yeah, I agree - maybe we should take a look at that assumption of 1 class == 1 function and re-evaluate if there's a better way.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dennisgove commented on the issue:

          https://github.com/apache/lucene-solr/pull/171

          I think this all looks good.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dennisgove commented on the issue: https://github.com/apache/lucene-solr/pull/171 I think this all looks good.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dennisgove commented on the issue:

          https://github.com/apache/lucene-solr/pull/171

          I noticed that solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java is still part of the PR. Is that intentional?

          Show
          githubbot ASF GitHub Bot added a comment - Github user dennisgove commented on the issue: https://github.com/apache/lucene-solr/pull/171 I noticed that solr/solrj/src/java/org/apache/solr/client/solrj/io/eval/DatePartEvaluator.java is still part of the PR. Is that intentional?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user covolution commented on the issue:

          https://github.com/apache/lucene-solr/pull/171

          Yes, I had some minor commits (including the delete) I hadn't pushed. It up to date now.

          Show
          githubbot ASF GitHub Bot added a comment - Github user covolution commented on the issue: https://github.com/apache/lucene-solr/pull/171 Yes, I had some minor commits (including the delete) I hadn't pushed. It up to date now.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r110244491

          — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java —
          @@ -191,7 +237,20 @@ public void inform(SolrCore core) {
          .withFunctionName("lteq", LessThanEqualToEvaluator.class)
          .withFunctionName("not", NotEvaluator.class)
          .withFunctionName("or", OrEvaluator.class)

          • +
            + // Date Time Evaluators
            + .withFunctionName(TemporalEvaluatorYear.FUNCTION_NAME, TemporalEvaluatorYear.class)

              • End diff –

          I'm having trouble making up my mind about this use of the FUNCTION_NAME static property. For a while I've been considering moving the function name into the implementing class (like you've done here) but I haven't been able to convince myself that it is inherently better. I'd like to discuss it though, so I'll list my reasons both for and against.

          1. Function names were originally assigned in a single place, the StreamHandler, to allow for easy overrides via the config file (eg. using business specific logic for an innerJoin you could override the assignment of `innerJoin` to your own class via the config file). Having the assignments in a single place made this easier.

          2. Having assignments in a single place makes it easy to see what the full list of available function names is. Among other things, this has helped prevent me from accidentally using an already used function name for a different class.

          3. Function names are now being assigned in at least 3 places ([StreamHandler](https://github.com/dennisgove/lucene-solr/blob/master/solr/core/src/java/org/apache/solr/handler/StreamHandler.java), [GraphHandler](https://github.com/dennisgove/lucene-solr/blob/master/solr/core/src/java/org/apache/solr/handler/GraphHandler.java), and [SolrTable](https://github.com/dennisgove/lucene-solr/blob/master/solr/core/src/java/org/apache/solr/handler/sql/SolrTable.java)) so all the benefits of a single assignment is somewhat out the window.

          4. By assigning the function names in the implementing class, we no longer need to rely on the StreamFactory for knowing what the assigned function name for a class is. This means we can use the function name in more logging and error messages where a StreamFactory instance may not be readily available.

          5. It will still be possible for people to override the default assigned function names and classes, but the logic that currently does that will have to be carefully changed to ensure full backward compatibility.

          I'm in favor of making this change, but it's larger than something that should probably be done in this PR.

          @joel-bernstein, @covolution - your thoughts?

          Show
          githubbot ASF GitHub Bot added a comment - Github user dennisgove commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r110244491 — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java — @@ -191,7 +237,20 @@ public void inform(SolrCore core) { .withFunctionName("lteq", LessThanEqualToEvaluator.class) .withFunctionName("not", NotEvaluator.class) .withFunctionName("or", OrEvaluator.class) + + // Date Time Evaluators + .withFunctionName(TemporalEvaluatorYear.FUNCTION_NAME, TemporalEvaluatorYear.class) End diff – I'm having trouble making up my mind about this use of the FUNCTION_NAME static property. For a while I've been considering moving the function name into the implementing class (like you've done here) but I haven't been able to convince myself that it is inherently better. I'd like to discuss it though, so I'll list my reasons both for and against. 1. Function names were originally assigned in a single place, the StreamHandler, to allow for easy overrides via the config file (eg. using business specific logic for an innerJoin you could override the assignment of `innerJoin` to your own class via the config file). Having the assignments in a single place made this easier. 2. Having assignments in a single place makes it easy to see what the full list of available function names is. Among other things, this has helped prevent me from accidentally using an already used function name for a different class. 3. Function names are now being assigned in at least 3 places ( [StreamHandler] ( https://github.com/dennisgove/lucene-solr/blob/master/solr/core/src/java/org/apache/solr/handler/StreamHandler.java ), [GraphHandler] ( https://github.com/dennisgove/lucene-solr/blob/master/solr/core/src/java/org/apache/solr/handler/GraphHandler.java ), and [SolrTable] ( https://github.com/dennisgove/lucene-solr/blob/master/solr/core/src/java/org/apache/solr/handler/sql/SolrTable.java )) so all the benefits of a single assignment is somewhat out the window. 4. By assigning the function names in the implementing class, we no longer need to rely on the StreamFactory for knowing what the assigned function name for a class is. This means we can use the function name in more logging and error messages where a StreamFactory instance may not be readily available. 5. It will still be possible for people to override the default assigned function names and classes, but the logic that currently does that will have to be carefully changed to ensure full backward compatibility. I'm in favor of making this change, but it's larger than something that should probably be done in this PR. @joel-bernstein, @covolution - your thoughts?
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r110346756

          — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java —
          @@ -191,7 +237,20 @@ public void inform(SolrCore core) {
          .withFunctionName("lteq", LessThanEqualToEvaluator.class)
          .withFunctionName("not", NotEvaluator.class)
          .withFunctionName("or", OrEvaluator.class)

          • +
            + // Date Time Evaluators
            + .withFunctionName(TemporalEvaluatorYear.FUNCTION_NAME, TemporalEvaluatorYear.class)

              • End diff –

          I agree with what you are saying. Its a tricky question.

          The function definitions in StreamHandler are all static, there are no class instances. This makes sense if a factory is creating them. If we went for an instance method on an implementation class then that wouldn't really work. (It would allow us to have multiple implementations in the same class though)!

          So instances vs static is a bit of a problem. I like that you can also use aliases at the moment so wouldn't want to lose that. Are function classes being created on-demand, per request?

          If we keep with the static implementation, perhaps we can use an interface called FunctionNames to put all the string constants? It would be one central definition list for StreamHandler, GraphHandler and SolrTable. Function implementation classes could still reference the FunctionNames in their implementation.

          Show
          githubbot ASF GitHub Bot added a comment - Github user covolution commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r110346756 — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java — @@ -191,7 +237,20 @@ public void inform(SolrCore core) { .withFunctionName("lteq", LessThanEqualToEvaluator.class) .withFunctionName("not", NotEvaluator.class) .withFunctionName("or", OrEvaluator.class) + + // Date Time Evaluators + .withFunctionName(TemporalEvaluatorYear.FUNCTION_NAME, TemporalEvaluatorYear.class) End diff – I agree with what you are saying. Its a tricky question. The function definitions in StreamHandler are all static, there are no class instances. This makes sense if a factory is creating them. If we went for an instance method on an implementation class then that wouldn't really work. (It would allow us to have multiple implementations in the same class though)! So instances vs static is a bit of a problem. I like that you can also use aliases at the moment so wouldn't want to lose that. Are function classes being created on-demand, per request? If we keep with the static implementation, perhaps we can use an interface called FunctionNames to put all the string constants? It would be one central definition list for StreamHandler, GraphHandler and SolrTable. Function implementation classes could still reference the FunctionNames in their implementation.
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/lucene-solr/pull/171#discussion_r111227848

          — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java —
          @@ -191,7 +237,20 @@ public void inform(SolrCore core) {
          .withFunctionName("lteq", LessThanEqualToEvaluator.class)
          .withFunctionName("not", NotEvaluator.class)
          .withFunctionName("or", OrEvaluator.class)

          • +
            + // Date Time Evaluators
            + .withFunctionName(TemporalEvaluatorYear.FUNCTION_NAME, TemporalEvaluatorYear.class)

              • End diff –

          I'd like to approach this in a separate ticket. Would it be alright if we change the use of TemporalEvaluatorYear.FUNCTION_NAME to "year" in the call to `withFunctionName` for now?

          Show
          githubbot ASF GitHub Bot added a comment - Github user dennisgove commented on a diff in the pull request: https://github.com/apache/lucene-solr/pull/171#discussion_r111227848 — Diff: solr/core/src/java/org/apache/solr/handler/StreamHandler.java — @@ -191,7 +237,20 @@ public void inform(SolrCore core) { .withFunctionName("lteq", LessThanEqualToEvaluator.class) .withFunctionName("not", NotEvaluator.class) .withFunctionName("or", OrEvaluator.class) + + // Date Time Evaluators + .withFunctionName(TemporalEvaluatorYear.FUNCTION_NAME, TemporalEvaluatorYear.class) End diff – I'd like to approach this in a separate ticket. Would it be alright if we change the use of TemporalEvaluatorYear.FUNCTION_NAME to "year" in the call to `withFunctionName` for now?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joel-bernstein commented on the issue:

          https://github.com/apache/lucene-solr/pull/171

          Hi @dennisgove,

          I've been working with this ticket today. I'm commit it to master so you and I can make changes more easily to it. I also introduced the concept of a TupleContext in this ticket which may need some re-working. When we feel it's ready we can backport.

          Show
          githubbot ASF GitHub Bot added a comment - Github user joel-bernstein commented on the issue: https://github.com/apache/lucene-solr/pull/171 Hi @dennisgove, I've been working with this ticket today. I'm commit it to master so you and I can make changes more easily to it. I also introduced the concept of a TupleContext in this ticket which may need some re-working. When we feel it's ready we can backport.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dennisgove commented on the issue:

          https://github.com/apache/lucene-solr/pull/171

          Sounds good to me. I like the work that was done here.

          Show
          githubbot ASF GitHub Bot added a comment - Github user dennisgove commented on the issue: https://github.com/apache/lucene-solr/pull/171 Sounds good to me. I like the work that was done here.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user joel-bernstein commented on the issue:

          https://github.com/apache/lucene-solr/pull/171

          Ok great. Just running the test suite now...

          Show
          githubbot ASF GitHub Bot added a comment - Github user joel-bernstein commented on the issue: https://github.com/apache/lucene-solr/pull/171 Ok great. Just running the test suite now...
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit c3d205cdccf1175d9960723d0fe4e0d4391dff11 in lucene-solr's branch refs/heads/master from Gethin James
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c3d205c ]

          SOLR-10303: Switching from the fieldName param to subEvaluators

          Show
          jira-bot ASF subversion and git services added a comment - Commit c3d205cdccf1175d9960723d0fe4e0d4391dff11 in lucene-solr's branch refs/heads/master from Gethin James [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c3d205c ] SOLR-10303 : Switching from the fieldName param to subEvaluators
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit cf14b4be034975417bbdc1185b5aef392c00ae91 in lucene-solr's branch refs/heads/master from Gethin James
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=cf14b4b ]

          SOLR-10303: Supporting epoch for LocalDateTime

          Show
          jira-bot ASF subversion and git services added a comment - Commit cf14b4be034975417bbdc1185b5aef392c00ae91 in lucene-solr's branch refs/heads/master from Gethin James [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=cf14b4b ] SOLR-10303 : Supporting epoch for LocalDateTime
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit c6fbb27376d7ac53e149da2e420bb81bdb2513be in lucene-solr's branch refs/heads/master from Gethin James
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c6fbb27 ]

          SOLR-10303: Supporting more datatypes via a TemporalAccessor

          Show
          jira-bot ASF subversion and git services added a comment - Commit c6fbb27376d7ac53e149da2e420bb81bdb2513be in lucene-solr's branch refs/heads/master from Gethin James [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=c6fbb27 ] SOLR-10303 : Supporting more datatypes via a TemporalAccessor
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit b314bc67764131bb677f25d98212577182af0b1e in lucene-solr's branch refs/heads/master from Gethin James
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b314bc6 ]

          SOLR-10303: Switched to pascal casing

          Show
          jira-bot ASF subversion and git services added a comment - Commit b314bc67764131bb677f25d98212577182af0b1e in lucene-solr's branch refs/heads/master from Gethin James [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b314bc6 ] SOLR-10303 : Switched to pascal casing
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit d70fc967d44fd0a7e3706707479853dfd43ea908 in lucene-solr's branch refs/heads/master from Gethin James
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d70fc96 ]

          SOLR-10303: Refactored to multiple TemporalEvaluator classes based on feedback

          Show
          jira-bot ASF subversion and git services added a comment - Commit d70fc967d44fd0a7e3706707479853dfd43ea908 in lucene-solr's branch refs/heads/master from Gethin James [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=d70fc96 ] SOLR-10303 : Refactored to multiple TemporalEvaluator classes based on feedback
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1c333c79d0cd3d94e85222030788bf5597732f2c in lucene-solr's branch refs/heads/master from Gethin James
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1c333c7 ]

          SOLR-10303: Removing the unused class, replaced by TemporalEvaluator

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1c333c79d0cd3d94e85222030788bf5597732f2c in lucene-solr's branch refs/heads/master from Gethin James [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1c333c7 ] SOLR-10303 : Removing the unused class, replaced by TemporalEvaluator
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 8642ed9f88e2c8667ff80835b2b3b632786884ab in lucene-solr's branch refs/heads/master from Gethin James
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8642ed9 ]

          SOLR-10303: Error message formatting for TemporalEvaluator

          Show
          jira-bot ASF subversion and git services added a comment - Commit 8642ed9f88e2c8667ff80835b2b3b632786884ab in lucene-solr's branch refs/heads/master from Gethin James [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8642ed9 ] SOLR-10303 : Error message formatting for TemporalEvaluator
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit b78a270c9d3f0040325bbe4eb044384173bff963 in lucene-solr's branch refs/heads/master from Gethin James
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b78a270 ]

          SOLR-10303: Removing the unused class DatePartEvaluator from the test

          Show
          jira-bot ASF subversion and git services added a comment - Commit b78a270c9d3f0040325bbe4eb044384173bff963 in lucene-solr's branch refs/heads/master from Gethin James [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b78a270 ] SOLR-10303 : Removing the unused class DatePartEvaluator from the test
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 5e403647de109685cbfc0c81943ae9e79638348f in lucene-solr's branch refs/heads/master from Joel Bernstein
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5e40364 ]

          SOLR-10303: Add the tuple context to avoid creating multiple LocalDateTime instances for the same Tuple

          Show
          jira-bot ASF subversion and git services added a comment - Commit 5e403647de109685cbfc0c81943ae9e79638348f in lucene-solr's branch refs/heads/master from Joel Bernstein [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5e40364 ] SOLR-10303 : Add the tuple context to avoid creating multiple LocalDateTime instances for the same Tuple
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 0bcd88b181b3f9d927ae5507b700321eb6e68af6 in lucene-solr's branch refs/heads/master from Joel Bernstein
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=0bcd88b ]

          SOLR-10303: Fix precommit

          Show
          jira-bot ASF subversion and git services added a comment - Commit 0bcd88b181b3f9d927ae5507b700321eb6e68af6 in lucene-solr's branch refs/heads/master from Joel Bernstein [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=0bcd88b ] SOLR-10303 : Fix precommit
          Hide
          joel.bernstein Joel Bernstein added a comment -

          Dennis Gove, it's all committed now. Feel free to change things around. Let me know when you've finished I can do the backport.

          Show
          joel.bernstein Joel Bernstein added a comment - Dennis Gove , it's all committed now. Feel free to change things around. Let me know when you've finished I can do the backport.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user covolution commented on the issue:

          https://github.com/apache/lucene-solr/pull/171

          Changes merged into master

          Show
          githubbot ASF GitHub Bot added a comment - Github user covolution commented on the issue: https://github.com/apache/lucene-solr/pull/171 Changes merged into master
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user covolution closed the pull request at:

          https://github.com/apache/lucene-solr/pull/171

          Show
          githubbot ASF GitHub Bot added a comment - Github user covolution closed the pull request at: https://github.com/apache/lucene-solr/pull/171
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 867e81672a42f2a202382367e0e546947a5e4946 in lucene-solr's branch refs/heads/master from Joel Bernstein
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=867e816 ]

          SOLR-10303: Update CHANGES.txt

          Show
          jira-bot ASF subversion and git services added a comment - Commit 867e81672a42f2a202382367e0e546947a5e4946 in lucene-solr's branch refs/heads/master from Joel Bernstein [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=867e816 ] SOLR-10303 : Update CHANGES.txt
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit eefd18553e765b62591a707cb8c049e2fa7b4573 in lucene-solr's branch refs/heads/branch_6x from Joel Bernstein
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=eefd185 ]

          SOLR-10303: Update CHANGES.txt

          Show
          jira-bot ASF subversion and git services added a comment - Commit eefd18553e765b62591a707cb8c049e2fa7b4573 in lucene-solr's branch refs/heads/branch_6x from Joel Bernstein [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=eefd185 ] SOLR-10303 : Update CHANGES.txt
          Hide
          joel.bernstein Joel Bernstein added a comment -

          Closing this ticket out as it is now backported for 6.6. Thanks Gethin James!

          Show
          joel.bernstein Joel Bernstein added a comment - Closing this ticket out as it is now backported for 6.6. Thanks Gethin James !

            People

            • Assignee:
              joel.bernstein Joel Bernstein
              Reporter:
              joel.bernstein Joel Bernstein
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development