Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Commit

Permalink
Support NULL literal as function argument (#985)
Browse files Browse the repository at this point in the history
* Add undefined type and its distance

* Add doctest

* Fix broken UT

* Make undefined compatible with any other type

* Make undefined compatible with other type by data type tree hiearchy

* Prepare PR

* Add IT and change JDBC driver to support undefined type

* Prepare PR

* Missing value has undefined type too

* Move doc to data type section

* Fix compile error after merge
  • Loading branch information
dai-chen authored Jan 15, 2021
1 parent 47d50d2 commit 6899363
Show file tree
Hide file tree
Showing 18 changed files with 181 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
@RequiredArgsConstructor
public enum DataType {
TYPE_ERROR(ExprCoreType.UNKNOWN),
NULL(ExprCoreType.UNKNOWN),
NULL(ExprCoreType.UNDEFINED),

INTEGER(ExprCoreType.INTEGER),
LONG(ExprCoreType.LONG),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType;
import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType;
import com.amazon.opendistroforelasticsearch.sql.exception.ExpressionEvaluationException;
import java.util.Objects;

/**
Expand All @@ -40,7 +39,7 @@ public Object value() {

@Override
public ExprType type() {
return ExprCoreType.UNKNOWN;
return ExprCoreType.UNDEFINED;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public Object value() {

@Override
public ExprType type() {
return ExprCoreType.UNKNOWN;
return ExprCoreType.UNDEFINED;
}

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

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
Expand All @@ -29,14 +30,21 @@
*/
public enum ExprCoreType implements ExprType {
/**
* UNKNOWN.
* Unknown due to unsupported data type.
*/
UNKNOWN,

/**
* Undefined type for special literal such as NULL.
* As the root of data type tree, it is compatible with any other type.
* In other word, undefined type is the "narrowest" type.
*/
UNDEFINED,

/**
* Numbers.
*/
BYTE,
BYTE(UNDEFINED),
SHORT(BYTE),
INTEGER(SHORT),
LONG(INTEGER),
Expand All @@ -46,38 +54,38 @@ public enum ExprCoreType implements ExprType {
/**
* Boolean.
*/
BOOLEAN,
BOOLEAN(UNDEFINED),

/**
* String.
*/
STRING,
STRING(UNDEFINED),


/**
* Date.
* Todo. compatible relationship.
*/
TIMESTAMP,
DATE,
TIME,
DATETIME,
INTERVAL,
TIMESTAMP(UNDEFINED),
DATE(UNDEFINED),
TIME(UNDEFINED),
DATETIME(UNDEFINED),
INTERVAL(UNDEFINED),

/**
* Struct.
*/
STRUCT,
STRUCT(UNDEFINED),

/**
* Array.
*/
ARRAY;
ARRAY(UNDEFINED);

/**
* Parent of current base type.
* Parents (wider/compatible types) of current base type.
*/
private ExprCoreType parent;
private final List<ExprType> parents = new ArrayList<>();

/**
* The mapping between Type and legacy JDBC type name.
Expand All @@ -91,13 +99,13 @@ public enum ExprCoreType implements ExprType {

ExprCoreType(ExprCoreType... compatibleTypes) {
for (ExprCoreType subType : compatibleTypes) {
subType.parent = this;
subType.parents.add(this);
}
}

@Override
public List<ExprType> getParent() {
return Arrays.asList(parent == null ? UNKNOWN : parent);
return parents.isEmpty() ? ExprType.super.getParent() : parents;
}

@Override
Expand All @@ -113,9 +121,11 @@ public String legacyTypeName() {
/**
* Return all the valid ExprCoreType.
*/
public static List<ExprType> coreTypes() {
return Arrays.stream(ExprCoreType.values()).filter(type -> type != UNKNOWN)
.collect(Collectors.toList());
public static List<ExprCoreType> coreTypes() {
return Arrays.stream(ExprCoreType.values())
.filter(type -> type != UNKNOWN)
.filter(type -> type != UNDEFINED)
.collect(Collectors.toList());
}

public static List<ExprType> numberTypes() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package com.amazon.opendistroforelasticsearch.sql.expression.conditional.cases;

import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.UNKNOWN;
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.UNDEFINED;

import com.amazon.opendistroforelasticsearch.sql.data.model.ExprNullValue;
import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue;
Expand Down Expand Up @@ -75,8 +75,8 @@ public ExprValue valueOf(Environment<Expression, ExprValue> valueEnv) {
public ExprType type() {
List<ExprType> types = allResultTypes();

// Return unknown if all WHEN/ELSE return NULL
return types.isEmpty() ? UNKNOWN : types.get(0);
// Return undefined if all WHEN/ELSE return NULL
return types.isEmpty() ? UNDEFINED : types.get(0);
}

@Override
Expand All @@ -98,7 +98,7 @@ public List<ExprType> allResultTypes() {
types.add(defaultResult.type());
}

types.removeIf(type -> (type == UNKNOWN));
types.removeIf(type -> (type == UNDEFINED));
return types;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ private static FunctionResolver isNotNull() {

private static FunctionResolver ifNull() {
FunctionName functionName = BuiltinFunctionName.IFNULL.getName();
List<ExprType> typeList = ExprCoreType.coreTypes();
List<ExprCoreType> typeList = ExprCoreType.coreTypes();

List<SerializableFunction<FunctionName, org.apache.commons.lang3.tuple.Pair<FunctionSignature,
FunctionBuilder>>> functionsOne = typeList.stream().map(v ->
Expand All @@ -121,7 +121,7 @@ private static FunctionResolver ifNull() {

private static FunctionResolver nullIf() {
FunctionName functionName = BuiltinFunctionName.NULLIF.getName();
List<ExprType> typeList = ExprCoreType.coreTypes();
List<ExprCoreType> typeList = ExprCoreType.coreTypes();

FunctionResolver functionResolver =
FunctionDSL.define(functionName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public void getValue() {

@Test
public void getType() {
assertEquals(ExprCoreType.UNKNOWN, LITERAL_MISSING.type());
assertEquals(ExprCoreType.UNDEFINED, LITERAL_MISSING.type());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public void getValue() {

@Test
public void getType() {
assertEquals(ExprCoreType.UNKNOWN, LITERAL_NULL.type());
assertEquals(ExprCoreType.UNDEFINED, LITERAL_NULL.type());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.SHORT;
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING;
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRUCT;
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.UNDEFINED;
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.UNKNOWN;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -51,6 +52,12 @@ public void isCompatible() {
assertFalse(INTEGER.isCompatible(UNKNOWN));
}

@Test
public void isCompatibleWithUndefined() {
ExprCoreType.coreTypes().forEach(type -> assertTrue(type.isCompatible(UNDEFINED)));
ExprCoreType.coreTypes().forEach(type -> assertFalse(UNDEFINED.isCompatible(type)));
}

@Test
public void getParent() {
assertThat(((ExprType) () -> "test").getParent(), Matchers.contains(UNKNOWN));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ void should_use_type_of_when_clause() {

@Test
void should_use_type_of_nonnull_when_or_else_clause() {
when(whenClause.type()).thenReturn(ExprCoreType.UNKNOWN);
when(whenClause.type()).thenReturn(ExprCoreType.UNDEFINED);
Expression defaultResult = mock(Expression.class);
when(defaultResult.type()).thenReturn(ExprCoreType.STRING);

Expand All @@ -87,12 +87,12 @@ void should_use_type_of_nonnull_when_or_else_clause() {

@Test
void should_use_unknown_type_of_if_all_when_and_else_return_null() {
when(whenClause.type()).thenReturn(ExprCoreType.UNKNOWN);
when(whenClause.type()).thenReturn(ExprCoreType.UNDEFINED);
Expression defaultResult = mock(Expression.class);
when(defaultResult.type()).thenReturn(ExprCoreType.UNKNOWN);
when(defaultResult.type()).thenReturn(ExprCoreType.UNDEFINED);

CaseClause caseClause = new CaseClause(ImmutableList.of(whenClause), defaultResult);
assertEquals(ExprCoreType.UNKNOWN, caseClause.type());
assertEquals(ExprCoreType.UNDEFINED, caseClause.type());
}

@Test
Expand All @@ -109,9 +109,9 @@ void should_return_all_result_types_including_default() {

@Test
void should_return_all_result_types_excluding_null_result() {
when(whenClause.type()).thenReturn(ExprCoreType.UNKNOWN);
when(whenClause.type()).thenReturn(ExprCoreType.UNDEFINED);
Expression defaultResult = mock(Expression.class);
when(defaultResult.type()).thenReturn(ExprCoreType.UNKNOWN);
when(defaultResult.type()).thenReturn(ExprCoreType.UNDEFINED);

CaseClause caseClause = new CaseClause(ImmutableList.of(whenClause), defaultResult);
assertEquals(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.INTEGER;
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.LONG;
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.SHORT;
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.UNKNOWN;
import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.UNDEFINED;
import static com.amazon.opendistroforelasticsearch.sql.data.type.WideningTypeRule.IMPOSSIBLE_WIDENING;
import static com.amazon.opendistroforelasticsearch.sql.data.type.WideningTypeRule.TYPE_EQUAL;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -33,10 +33,9 @@
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.Lists;
import com.google.common.collect.Table;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
Expand All @@ -60,12 +59,16 @@ class WideningTypeRuleTest {
.put(LONG, FLOAT, 1)
.put(LONG, DOUBLE, 2)
.put(FLOAT, DOUBLE, 1)
.put(UNDEFINED, BYTE, 1)
.put(UNDEFINED, SHORT, 2)
.put(UNDEFINED, INTEGER, 3)
.put(UNDEFINED, LONG, 4)
.put(UNDEFINED, FLOAT, 5)
.put(UNDEFINED, DOUBLE, 6)
.build();

private static Stream<Arguments> distanceArguments() {
List<ExprCoreType> exprTypes =
Arrays.asList(ExprCoreType.values()).stream().filter(type -> type != UNKNOWN).collect(
Collectors.toList());
List<ExprCoreType> exprTypes = ExprCoreType.coreTypes();
return Lists.cartesianProduct(exprTypes, exprTypes).stream()
.map(list -> {
ExprCoreType type1 = list.get(0);
Expand All @@ -81,9 +84,7 @@ private static Stream<Arguments> distanceArguments() {
}

private static Stream<Arguments> validMaxTypes() {
List<ExprCoreType> exprTypes =
Arrays.asList(ExprCoreType.values()).stream().filter(type -> type != UNKNOWN).collect(
Collectors.toList());
List<ExprCoreType> exprTypes = ExprCoreType.coreTypes();
return Lists.cartesianProduct(exprTypes, exprTypes).stream()
.map(list -> {
ExprCoreType type1 = list.get(0);
Expand Down Expand Up @@ -117,4 +118,13 @@ public void max(ExprCoreType v1, ExprCoreType v2, ExprCoreType expected) {
assertEquals(expected, WideningTypeRule.max(v1, v2));
}
}

@Test
public void maxOfUndefinedAndOthersShouldBeTheOtherType() {
ExprCoreType.coreTypes().forEach(type ->
assertEquals(type, WideningTypeRule.max(type, UNDEFINED)));
ExprCoreType.coreTypes().forEach(type ->
assertEquals(type, WideningTypeRule.max(UNDEFINED, type)));
}

}
17 changes: 16 additions & 1 deletion docs/user/general/datatypes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,24 @@ The table below list the mapping between Elasticsearch Data Type, ODFE SQL Data
| nested | array | STRUCT |
+--------------------+---------------+-----------+

Notes: Not all the ODFE SQL Type has correspond Elasticsearch Type. e.g. data and time. To use function which required such data type, user should explict convert the data type.
Notes: Not all the ODFE SQL Type has correspond Elasticsearch Type. e.g. data and time. To use function which required such data type, user should explicitly convert the data type.


Undefined Data Type
===================

The type of a null literal is special and different from any existing one. In this case, an ``UNDEFINED`` type is in use when the type cannot be inferred at "compile time" (during query parsing and analyzing). The corresponding SQL type is NULL according to JDBC specification. Because this undefined type is compatible with any other type by design, a null literal can be accepted as a valid operand or function argument.

Here are examples for NULL literal and expressions with NULL literal involved::

od> SELECT NULL, NULL = NULL, 1 + NULL, LENGTH(NULL);
fetched rows / total rows = 1/1
+--------+---------------+------------+----------------+
| NULL | NULL = NULL | 1 + NULL | LENGTH(NULL) |
|--------+---------------+------------+----------------|
| null | null | null | null |
+--------+---------------+------------+----------------+


Numeric Data Types
==================
Expand Down
2 changes: 2 additions & 0 deletions docs/user/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ Open Distro for Elasticsearch SQL enables you to extract insights out of Elastic

- `Data Types <general/datatypes.rst>`_

- `Values <general/values.rst>`_

* **Data Query Language**

- `Expressions <dql/expressions.rst>`_
Expand Down
Loading

0 comments on commit 6899363

Please sign in to comment.