Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
matriv committed Jan 22, 2019
1 parent 3ea4bdf commit ffdfc3f
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 19 deletions.
9 changes: 5 additions & 4 deletions docs/reference/sql/functions/grouping.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ Instead one can rewrite the query to move the expression on the histogram _insid
include-tagged::{sql-specs}/docs.csv-spec[histogramDateTimeExpression]
----

[NOTE]
When the histogram in SQL is applied on **DATE** type instead of **DATETIME**, then the interval specified is rounded to
the nearest multiple of a day. E.g.: for `HISTOGRAM(CAST(birth_date AS DATE), INTERVAL '2 3:04' DAY TO MINUTE)`
the interval actually used will be `INTERVAL 2 DAYS`.
[IMPORTANT]
When the histogram in SQL is applied on **DATE** type instead of **DATETIME**, the interval specified is truncated to
the multiple of a day. E.g.: for `HISTOGRAM(CAST(birth_date AS DATE), INTERVAL '2 3:04' DAY TO MINUTE)` the interval
actually used will be `INTERVAL '2' DAY`. If the interval specified is less than 1 day, e.g.:
`HISTOGRAM(CAST(birth_date AS DATE), INTERVAL '20' HOUR)` then the interval used will be `INTERVAL '1' DAY`.
13 changes: 0 additions & 13 deletions x-pack/plugin/sql/qa/src/main/resources/date.csv-spec
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,6 @@ SELECT birth_date, last_name FROM "test_emp" WHERE birth_date <= CAST('1955-01-2
1952-04-19T00:00:00Z |Peac
;


// The date return can differ due to the randomized timezones (it's transformed to the JDBC client's timezone)
dateAsGroupingKey-Ignore
SELECT CAST(birth_date AS DATE) AS d, CAST(SUM(emp_no) AS INT) s FROM test_emp GROUP BY d ORDER BY d LIMIT 5;

d:date | s:i
null |100445
1952-02-26 |10097
1952-04-18 |10009
1952-05-14 |10072
1952-06-12 |10076
;

dateAndFunctionAsGroupingKey
SELECT MONTH(CAST(birth_date AS DATE)) AS m, CAST(SUM(emp_no) AS INT) s FROM test_emp GROUP BY m ORDER BY m LIMIT 5;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

package org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic;

import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor.BinaryArithmeticOperation;
import org.elasticsearch.xpack.sql.tree.Source;
Expand All @@ -14,6 +15,7 @@
import org.elasticsearch.xpack.sql.type.DataTypes;

import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
import static org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor.BinaryArithmeticOperation.SUB;

abstract class DateTimeArithmeticOperation extends ArithmeticOperation {

Expand Down Expand Up @@ -45,6 +47,9 @@ protected TypeResolution resolveType() {
if (DataTypeConversion.commonType(l, r) == null) {
return new TypeResolution(format("[{}] has arguments with incompatible types [{}] and [{}]", symbol(), l, r));
} else {
if (function() == SUB && right().dataType().isDateBased() && DataTypes.isInterval(left().dataType())) {
throw new SqlIllegalArgumentException("Cannot subtract a date from an interval; do you mean the reverse?");
}
return TypeResolution.TYPE_RESOLVED;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,18 @@ public static DataType commonType(DataType left, DataType right) {
}
}
if (left == DATETIME) {
if (right == DATE || DataTypes.isInterval(right)) {
if (right == DATE) {
return left;
}
if (DataTypes.isInterval(right)) {
return left;
}
}
if (right == DATETIME) {
if (left == DATE || DataTypes.isInterval(left)) {
if (left == DATE) {
return right;
}
if (DataTypes.isInterval(left)) {
return right;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,14 @@ public void testExtractNonDateTime() {
assertEquals("1:8: Invalid datetime field [ABS]. Use any datetime function.", error("SELECT EXTRACT(ABS FROM date) FROM test"));
}

public void testSubtractFromInterval() {
Analyzer analyzer = new Analyzer(TestUtils.TEST_CFG, new FunctionRegistry(), indexResolution, new Verifier(new Metrics()));
SqlIllegalArgumentException e = expectThrows(SqlIllegalArgumentException.class, () -> analyzer.analyze(parser.createStatement(
"SELECT INTERVAL 1 MONTH - CAST('2000-01-01' AS DATETIME)"), true));
assertEquals("Cannot subtract a date from an interval; do you mean the reverse?",
e.getMessage());
}

public void testMultipleColumns() {
// xxx offset is that of the order by field
assertEquals("1:43: Unknown column [xxx]\nline 1:8: Unknown column [xxx]",
Expand Down

0 comments on commit ffdfc3f

Please sign in to comment.