Druid SQL EXTRACT time function - adding support for additional Time Units #8068

sashidhar · 2019-07-12T06:41:44Z

Fixes #7935.

Description

Added support for MILLISECOND, MICROSECOND, ISODOW, ISOYEAR, DECADE, CENTURY and MILLENNIUM Time Units for EXTRACT time function.

Test case

Added a test case for testing extract time function covering above time units.

Documentation

Made corresponding documentation changes.

…for MILLISECOND 3. Added a test case to test extracting millisecond from expression. apache#7935

clintropolis · 2019-07-12T06:58:04Z

sql/src/test/java/org/apache/druid/sql/calcite/http/SqlResourceTest.java

@@ -689,6 +689,18 @@ private void checkSqlRequestLog(boolean success)
    }
  }

+  @Test
+  public void testExtractMillisecondFromExpr() throws Exception


I think CalciteQueryTests might be a more appropriate place to test this, maybe have a look at CalciteQueryTest.testFilterOnTimeExtract?

Thanks @clintropolis for the pointer. Moved this test to CalciteQueryTest. Added testFilterOnTimeExtractWithMilliseconds() in CalciteQueryTest. If the code changes look fine I'll go ahead and handle other time units. Introduced new data source DATASOURCE4 in tests to avoid polluting existing datasources (this is useful to test other time units as well).

Overall seems reasonable to me 👍. You could probably test most/all of the additions in a single test if the timestamps for the datasource you added are sufficient, if you'd like to avoid repeating a lot of the near same boilerplate in the calcite tests.

👍makes sense.

…conds

sashidhar · 2019-07-14T01:41:22Z

How do I fix this error ? The test was successful before I rebased changes from master. Post rebasing it failed. Trying to figure out how to fix this.

https://travis-ci.org/apache/incubator-druid/jobs/558313543

[ERROR] testFilterOnTimeExtractWithMilliseconds(org.apache.druid.sql.calcite.CalciteQueryTest) Time elapsed: 0.055 s <<< ERROR!
org.apache.druid.java.util.common.ISE: Cannot vectorize!
at org.apache.druid.query.QueryContexts$Vectorize$3.shouldVectorize(QueryContexts.java:77)
at org.apache.druid.query.timeseries.TimeseriesQueryEngine.process(TimeseriesQueryEngine.java:94)
at org.apache.druid.query.timeseries.TimeseriesQueryRunnerFactory$TimeseriesQueryRunner.run(TimeseriesQueryRunnerFactory.java:103)
at org.apache.druid.query.spec.SpecificSegmentQueryRunner.lambda$run$0(SpecificSegmentQueryRunner.java:73)
at org.apache.druid.query.spec.SpecificSegmentQueryRunner.doNamed(SpecificSegmentQueryRunner.java:168)
at org.apache.druid.query.spec.SpecificSegmentQueryRunner.run(SpecificSegmentQueryRunner.java:69)
at org.apache.druid.query.ChainedExecutionQueryRunner$1$1.call(ChainedExecutionQueryRunner.java:119)
at org.apache.druid.query.ChainedExecutionQueryRunner$1$1.call(ChainedExecutionQueryRunner.java:114)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at org.apache.druid.java.util.common.concurrent.DirectExecutorService.execute(DirectExecutorService.java:81)
at java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:134)
at com.google.common.util.concurrent.AbstractListeningExecutorService.submit(AbstractListeningExecutorService.java:58)
at org.apache.druid.query.ChainedExecutionQueryRunner$1.lambda$make$0(ChainedExecutionQueryRunner.java:112)
at com.google.common.collect.Iterators$8.transform(Iterators.java:794)
at com.google.common.collect.TransformedIterator.next(TransformedIterator.java:48)
at com.google.common.collect.Iterators.addAll(Iterators.java:357)
at com.google.common.collect.Lists.newArrayList(Lists.java:147)
at com.google.common.collect.Lists.newArrayList(Lists.java:129)
at org.apache.druid.query.ChainedExecutionQueryRunner$1.make(ChainedExecutionQueryRunner.java:104)
at org.apache.druid.java.util.common.guava.BaseSequence.accumulate(BaseSequence.java:42)
at org.apache.druid.common.guava.CombiningSequence.accumulate(CombiningSequence.java:64)
at org.apache.druid.java.util.common.guava.MappedSequence.accumulate(MappedSequence.java:43)
at org.apache.druid.common.guava.CombiningSequence.accumulate(CombiningSequence.java:64)
at org.apache.druid.java.util.common.guava.MappedSequence.accumulate(MappedSequence.java:43)
at org.apache.druid.java.util.common.guava.WrappingSequence$1.get(WrappingSequence.java:50)
at org.apache.druid.java.util.common.guava.SequenceWrapper.wrap(SequenceWrapper.java:55)
at org.apache.druid.java.util.common.guava.WrappingSequence.accumulate(WrappingSequence.java:45)
at org.apache.druid.java.util.common.guava.MappedSequence.accumulate(MappedSequence.java:43)
at org.apache.druid.java.util.common.guava.WrappingSequence$1.get(WrappingSequence.java:50)
at org.apache.druid.java.util.common.guava.SequenceWrapper.wrap(SequenceWrapper.java:55)
at org.apache.druid.java.util.common.guava.WrappingSequence.accumulate(WrappingSequence.java:45)
at org.apache.druid.java.util.common.guava.Sequence.toList(Sequence.java:85)
at org.apache.druid.sql.calcite.BaseCalciteQueryTest.getResults(BaseCalciteQueryTest.java:619)
at org.apache.druid.sql.calcite.BaseCalciteQueryTest.getResults(BaseCalciteQueryTest.java:566)
at org.apache.druid.sql.calcite.BaseCalciteQueryTest.testQuery(BaseCalciteQueryTest.java:554)
at org.apache.druid.sql.calcite.BaseCalciteQueryTest.testQuery(BaseCalciteQueryTest.java:481)
at org.apache.druid.sql.calcite.CalciteQueryTest.testFilterOnTimeExtractWithMilliseconds(CalciteQueryTest.java:5807)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.apache.druid.sql.calcite.util.QueryLogHook$1.evaluate(QueryLogHook.java:95)
at org.junit.rules.ExpectedException$ExpectedExceptionStatement.evaluate(ExpectedException.java:239)
at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
at org.junit.rules.RunRules.evaluate(RunRules.java:20)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:290)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)

....
....

[ERROR] org.apache.druid.sql.calcite.CalciteQueryTest.testFilterOnTimeExtractWithMilliseconds(org.apache.druid.sql.calcite.CalciteQueryTest)
[ERROR] Run 1: CalciteQueryTest.testFilterOnTimeExtractWithMilliseconds:5807->BaseCalciteQueryTest.testQuery:481->BaseCalciteQueryTest.testQuery:554->BaseCalciteQueryTest.getResults:566->BaseCalciteQueryTest.getResults:619 » ISE

clintropolis · 2019-07-14T02:06:07Z

How do I fix this error ? The test was successful before I rebased changes from master. Post rebasing it failed. Trying to figure out how to fix this.

Ah, add this to the beginning of your test

    cannotVectorize();

A vectorized query engine got merged, but it doesn't fully support all query and filter types, so some tests, such as those which use Druid expressions as this, will need that until supported.

…AR and CENTURY time units, documentation changes.

sashidhar · 2019-07-15T17:19:43Z

Changes made

Added support for MICROSECOND, MILLISECOND, ISODOW, ISOYEAR and CENTURY time units
A test case covering these time units.
Documentation changes.

Pending
I'm not sure what is the expected right expression/formula for DECADE and MILLENNIUM time units. Can someone help me with the right library/API methods to use ?

Timestamp used in the test 2000-01-01T10:51:45.695Z
What should be the DECADE here ?
Year 2000 falls under 2nd MILLENNIUM ?

…ation

sashidhar · 2019-07-18T19:24:20Z

Added support for the remaining time units i.e DECADE and MILLENNIUM. With this support for all missing time units is handled. Updated the test case accordingly. If the overall changes including the expression evaluation logic looks fine, I'll add and/or update more tests as needed.

clintropolis

lgtm 👍, thanks!

1. Added TimestampExtractExprMacro.Unit for MILLISECOND 2. expr eval …

c9fde6c

…for MILLISECOND 3. Added a test case to test extracting millisecond from expression. apache#7935

clintropolis reviewed Jul 12, 2019

View reviewed changes

clintropolis added Area - SQL Bug labels Jul 12, 2019

t-sashidhar added 4 commits July 13, 2019 10:20

1. Adding DATASOURCE4 in tests. 2. Adding test TimeExtractWithMillise…

4697b1d

…conds

Fixing testInformationSchemaTables test

6fd5d27

Merge branch 'master' into druid_sql

baa0e43

Fixing failing tests in DruidAvaticaHandlerTest

32a1140

t-sashidhar added 2 commits July 14, 2019 08:36

Adding cannotVectorize() call before the test

d74aaac

Extract time function - Adding support for MICROSECOND, ISODOW, ISOYE…

f39bc10

…AR and CENTURY time units, documentation changes.

t-sashidhar added 5 commits July 15, 2019 23:19

Merge branch 'master' into druid_sql

c4c3563

Adding MILLISECOND in test case

5180f33

Adding support DECADE and MILLENNIUM, updating test case and document…

ebc3cad

…ation

Merge branch 'master' into druid_sql

6f241f1

Fixing expression eval for DECADE and MILLENIUM

b246f8e

gianm requested a review from clintropolis July 19, 2019 21:17

clintropolis approved these changes Jul 20, 2019

View reviewed changes

clintropolis merged commit ea4bad7 into apache:master Jul 20, 2019

sashidhar deleted the druid_sql branch July 20, 2019 09:51

clintropolis added this to the 0.16.0 milestone Aug 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Druid SQL EXTRACT time function - adding support for additional Time Units #8068

Druid SQL EXTRACT time function - adding support for additional Time Units #8068

sashidhar commented Jul 12, 2019 •

edited

Loading

clintropolis Jul 12, 2019

sashidhar Jul 13, 2019 •

edited

Loading

clintropolis Jul 14, 2019

sashidhar Jul 14, 2019

sashidhar commented Jul 14, 2019 •

edited

Loading

clintropolis commented Jul 14, 2019

sashidhar commented Jul 15, 2019

sashidhar commented Jul 18, 2019 •

edited

Loading

clintropolis left a comment

Druid SQL EXTRACT time function - adding support for additional Time Units #8068

Druid SQL EXTRACT time function - adding support for additional Time Units #8068

Conversation

sashidhar commented Jul 12, 2019 • edited Loading

Description

Test case

Documentation

clintropolis Jul 12, 2019

Choose a reason for hiding this comment

sashidhar Jul 13, 2019 • edited Loading

Choose a reason for hiding this comment

clintropolis Jul 14, 2019

Choose a reason for hiding this comment

sashidhar Jul 14, 2019

Choose a reason for hiding this comment

sashidhar commented Jul 14, 2019 • edited Loading

clintropolis commented Jul 14, 2019

sashidhar commented Jul 15, 2019

sashidhar commented Jul 18, 2019 • edited Loading

clintropolis left a comment

Choose a reason for hiding this comment

sashidhar commented Jul 12, 2019 •

edited

Loading

sashidhar Jul 13, 2019 •

edited

Loading

sashidhar commented Jul 14, 2019 •

edited

Loading

sashidhar commented Jul 18, 2019 •

edited

Loading