Fix timestamp extract fn to match postgreSQL (#9337)

* Fix timestamp extract fn to match postgres

Update the timestamp extract function so that it matches the PostgreSQL docs.
Examples from the PostgreSQL docs were added as tests for DECADE, CENTURY
and MILLENIUM extraction.

There were bugs in CENTURY and MILLENIUM that were spotted because of intelliJ
inspections - 'Integer division in floating point context'

* Update CalciteQueryTest

* remove useless round

* mark integer division as an error
This commit is contained in:
Suneet Saldanha 2020-02-12 15:39:19 -08:00 committed by GitHub
parent c30579e47b
commit b1f38131af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 110 additions and 5 deletions

View File

@ -89,6 +89,7 @@
<inspection_tool class="InnerClassReferencedViaSubclass" enabled="true" level="ERROR" enabled_by_default="true" /> <inspection_tool class="InnerClassReferencedViaSubclass" enabled="true" level="ERROR" enabled_by_default="true" />
<inspection_tool class="InstanceofIncompatibleInterface" enabled="true" level="ERROR" enabled_by_default="true" /> <inspection_tool class="InstanceofIncompatibleInterface" enabled="true" level="ERROR" enabled_by_default="true" />
<inspection_tool class="InstantiationOfUtilityClass" enabled="true" level="ERROR" enabled_by_default="true" /> <inspection_tool class="InstantiationOfUtilityClass" enabled="true" level="ERROR" enabled_by_default="true" />
<inspection_tool class="IntegerDivisionInFloatingPointContext" enabled="true" level="ERROR" enabled_by_default="true" />
<inspection_tool class="InvalidComparatorMethodReference" enabled="true" level="ERROR" enabled_by_default="true" /> <inspection_tool class="InvalidComparatorMethodReference" enabled="true" level="ERROR" enabled_by_default="true" />
<inspection_tool class="IteratorHasNextCallsIteratorNext" enabled="true" level="ERROR" enabled_by_default="true" /> <inspection_tool class="IteratorHasNextCallsIteratorNext" enabled="true" level="ERROR" enabled_by_default="true" />
<inspection_tool class="IteratorNextDoesNotThrowNoSuchElementException" enabled="true" level="WARNING" enabled_by_default="true" /> <inspection_tool class="IteratorNextDoesNotThrowNoSuchElementException" enabled="true" level="WARNING" enabled_by_default="true" />

View File

@ -141,13 +141,13 @@ public class TimestampExtractExprMacro implements ExprMacroTable.ExprMacro
return ExprEval.of(dateTime.year().get()); return ExprEval.of(dateTime.year().get());
case DECADE: case DECADE:
// The year field divided by 10, See https://www.postgresql.org/docs/10/functions-datetime.html // The year field divided by 10, See https://www.postgresql.org/docs/10/functions-datetime.html
return ExprEval.of(Math.floor(dateTime.year().get() / 10)); return ExprEval.of(dateTime.year().get() / 10);
case CENTURY: case CENTURY:
return ExprEval.of(dateTime.centuryOfEra().get() + 1); return ExprEval.of(Math.ceil((double) dateTime.year().get() / 100));
case MILLENNIUM: case MILLENNIUM:
// Years in the 1900s are in the second millennium. The third millennium started January 1, 2001. // Years in the 1900s are in the second millennium. The third millennium started January 1, 2001.
// See https://www.postgresql.org/docs/10/functions-datetime.html // See https://www.postgresql.org/docs/10/functions-datetime.html
return ExprEval.of(Math.round(Math.ceil(dateTime.year().get() / 1000))); return ExprEval.of(Math.ceil((double) dateTime.year().get() / 1000));
default: default:
throw new ISE("Unhandled unit[%s]", unit); throw new ISE("Unhandled unit[%s]", unit);
} }

View File

@ -0,0 +1,102 @@
/*
* 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.druid.query.expression;
import com.google.common.collect.ImmutableList;
import org.apache.druid.common.config.NullHandling;
import org.apache.druid.math.expr.Expr;
import org.apache.druid.math.expr.ExprEval;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
/**
* These tests are copied from examples here -
* https://www.postgresql.org/docs/10/functions-datetime.html
*/
public class TimestampExtractExprMacroTest
{
static {
NullHandling.initializeForTests();
}
private TimestampExtractExprMacro target;
@Before
public void setUp()
{
target = new TimestampExtractExprMacro();
}
@Test
public void testApplyExtractDecadeShouldExtractTheCorrectDecade()
{
Expr expression = target.apply(
ImmutableList.of(
ExprEval.of("2001-02-16").toExpr(),
ExprEval.of(TimestampExtractExprMacro.Unit.DECADE.toString()).toExpr()
));
Assert.assertEquals(200, expression.eval(ExprUtils.nilBindings()).asInt());
}
@Test
public void testApplyExtractCenturyShouldExtractTheCorrectCentury()
{
Expr expression = target.apply(
ImmutableList.of(
ExprEval.of("2000-12-16").toExpr(),
ExprEval.of(TimestampExtractExprMacro.Unit.CENTURY.toString()).toExpr()
));
Assert.assertEquals(20, expression.eval(ExprUtils.nilBindings()).asInt());
}
@Test
public void testApplyExtractCenturyShouldBeTwentyFirstCenturyIn2001()
{
Expr expression = target.apply(
ImmutableList.of(
ExprEval.of("2001-02-16").toExpr(),
ExprEval.of(TimestampExtractExprMacro.Unit.CENTURY.toString()).toExpr()
));
Assert.assertEquals(21, expression.eval(ExprUtils.nilBindings()).asInt());
}
@Test
public void testApplyExtractMilleniumShouldExtractTheCorrectMillenium()
{
Expr expression = target.apply(
ImmutableList.of(
ExprEval.of("2000-12-16").toExpr(),
ExprEval.of(TimestampExtractExprMacro.Unit.MILLENNIUM.toString()).toExpr()
));
Assert.assertEquals(2, expression.eval(ExprUtils.nilBindings()).asInt());
}
@Test
public void testApplyExtractMilleniumShouldBeThirdMilleniumIn2001()
{
Expr expression = target.apply(
ImmutableList.of(
ExprEval.of("2001-02-16").toExpr(),
ExprEval.of(TimestampExtractExprMacro.Unit.MILLENNIUM.toString()).toExpr()
));
Assert.assertEquals(3, expression.eval(ExprUtils.nilBindings()).asInt());
}
}

View File

@ -65,6 +65,8 @@ public class CachingCostBalancerStrategy extends CostBalancerStrategy
return Double.POSITIVE_INFINITY; return Double.POSITIVE_INFINITY;
} }
// This is probably intentional. The tests fail if this is changed to floating point division.
//noinspection IntegerDivisionInFloatingPointContext
return cost * (server.getMaxSize() / server.getAvailableSize()); return cost * (server.getMaxSize() / server.getAvailableSize());
} }

View File

@ -7954,7 +7954,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
+ "AND EXTRACT(ISODOW FROM __time) = 6\n" + "AND EXTRACT(ISODOW FROM __time) = 6\n"
+ "AND EXTRACT(ISOYEAR FROM __time) = 2000\n" + "AND EXTRACT(ISOYEAR FROM __time) = 2000\n"
+ "AND EXTRACT(DECADE FROM __time) = 200\n" + "AND EXTRACT(DECADE FROM __time) = 200\n"
+ "AND EXTRACT(CENTURY FROM __time) = 21\n" + "AND EXTRACT(CENTURY FROM __time) = 20\n"
+ "AND EXTRACT(MILLENNIUM FROM __time) = 2\n", + "AND EXTRACT(MILLENNIUM FROM __time) = 2\n",
TIMESERIES_CONTEXT_DEFAULT, TIMESERIES_CONTEXT_DEFAULT,
@ -7982,7 +7982,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
selector("v3", "6", null), selector("v3", "6", null),
selector("v4", "2000", null), selector("v4", "2000", null),
selector("v5", "200", null), selector("v5", "200", null),
selector("v6", "21", null), selector("v6", "20", null),
selector("v7", "2", null) selector("v7", "2", null)
) )
) )