From 1e143f63cbb9ad39097e9bb1ff1cbe16b57c8fd8 Mon Sep 17 00:00:00 2001
From: Marios Trivyzas <matriv@gmail.com>
Date: Wed, 11 Sep 2019 18:27:04 +0300
Subject: [PATCH] SQL: Fix issue with common type resolution (#46565)

Many scalar functions try to find out the common type between their
arguments in order to set it as their return time, e.g.:
for `float + double` the common type which is set as the return type
of the + operation is `double`.

Previously, for data types TEXT and KEYWORD (string data types) there
was no common data type found and null was returned causing NPEs when
the function was trying to resolve the return data type.

Fixes: #46551
(cherry picked from commit 291017d69dfc810707c3c7c692f5a50af431b790)
---
 .../elasticsearch/xpack/sql/type/DataTypeConversion.java   | 7 +++++++
 .../xpack/sql/type/DataTypeConversionTests.java            | 5 ++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java
index 5fd1867aeb27a..ebae0a516a4de 100644
--- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java
+++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java
@@ -23,6 +23,7 @@
 import static org.elasticsearch.xpack.sql.type.DataType.DATETIME;
 import static org.elasticsearch.xpack.sql.type.DataType.LONG;
 import static org.elasticsearch.xpack.sql.type.DataType.NULL;
+import static org.elasticsearch.xpack.sql.type.DataType.TEXT;
 import static org.elasticsearch.xpack.sql.type.DataType.TIME;
 
 /**
@@ -50,6 +51,12 @@ public static DataType commonType(DataType left, DataType right) {
         if (DataTypes.isNull(right)) {
             return left;
         }
+        if (left.isString() && right.isString()) {
+            if (left == TEXT) {
+                return TEXT;
+            }
+            return right;
+        }
         if (left.isNumeric() && right.isNumeric()) {
             // if one is int
             if (left.isInteger()) {
diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java
index 1e4a61265eceb..ea5f9efc0bec9 100644
--- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java
+++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java
@@ -621,12 +621,15 @@ public void testCommonType() {
         assertEquals(NULL, commonType(NULL, NULL));
         assertEquals(INTEGER, commonType(INTEGER, KEYWORD));
         assertEquals(LONG, commonType(TEXT, LONG));
-        assertNull(commonType(TEXT, KEYWORD));
         assertEquals(SHORT, commonType(SHORT, BYTE));
         assertEquals(FLOAT, commonType(BYTE, FLOAT));
         assertEquals(FLOAT, commonType(FLOAT, INTEGER));
         assertEquals(DOUBLE, commonType(DOUBLE, FLOAT));
 
+        // strings
+        assertEquals(TEXT, commonType(TEXT, KEYWORD));
+        assertEquals(TEXT, commonType(KEYWORD, TEXT));
+
         // numeric and intervals
         assertEquals(INTERVAL_YEAR_TO_MONTH, commonType(INTERVAL_YEAR_TO_MONTH, LONG));
         assertEquals(INTERVAL_HOUR_TO_MINUTE, commonType(INTEGER, INTERVAL_HOUR_TO_MINUTE));