SQL: Fix bug regarding alias fields with dots (#37279)

Field of types aliases that have dots in name are returned without a
hierarchy by field_caps, as oppose to the mapping api or field with
concrete types, which in turn breaks IndexResolver.
This commit fixes this by creating the backing hierarchy similar to the
mapping api.

Close #37224
This commit is contained in:
Costin Leau 2019-01-10 22:18:53 +02:00 committed by GitHub
parent a2d63ecdc0
commit 83f7423cd6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 190 additions and 13 deletions

View File

@ -42,6 +42,7 @@ public abstract class CsvSpecTestCase extends SpecBaseIntegrationTestCase {
tests.addAll(readScriptSpec("/nested.csv-spec", parser));
tests.addAll(readScriptSpec("/functions.csv-spec", parser));
tests.addAll(readScriptSpec("/math.csv-spec", parser));
tests.addAll(readScriptSpec("/field-alias.csv-spec", parser));
return tests;
}

View File

@ -105,6 +105,10 @@ public class DataLoader {
if (extraFields) {
createIndex.startObject("extra_gender").field("type", "keyword").endObject();
createIndex.startObject("extra.info.gender")
.field("type", "alias")
.field("path", "gender")
.endObject();
}
createIndex.startObject("birth_date").field("type", "date").endObject();

View File

@ -36,6 +36,9 @@ dep.dep_name.keyword|VARCHAR |KEYWORD
dep.from_date |TIMESTAMP |DATE
dep.to_date |TIMESTAMP |DATE
emp_no |INTEGER |INTEGER
extra |STRUCT |OBJECT
extra.info |STRUCT |OBJECT
extra.info.gender |VARCHAR |KEYWORD
extra_gender |VARCHAR |KEYWORD
extra_no |INTEGER |INTEGER
first_name |VARCHAR |TEXT
@ -45,7 +48,7 @@ hire_date |TIMESTAMP |DATE
languages |TINYINT |BYTE
last_name |VARCHAR |TEXT
last_name.keyword |VARCHAR |KEYWORD
salary |INTEGER |INTEGER
salary |INTEGER |INTEGER
;
describePattern
@ -61,6 +64,9 @@ dep.dep_name.keyword|VARCHAR |KEYWORD
dep.from_date |TIMESTAMP |DATE
dep.to_date |TIMESTAMP |DATE
emp_no |INTEGER |INTEGER
extra |STRUCT |OBJECT
extra.info |STRUCT |OBJECT
extra.info.gender |VARCHAR |KEYWORD
extra_gender |VARCHAR |KEYWORD
extra_no |INTEGER |INTEGER
first_name |VARCHAR |TEXT
@ -70,7 +76,7 @@ hire_date |TIMESTAMP |DATE
languages |TINYINT |BYTE
last_name |VARCHAR |TEXT
last_name.keyword |VARCHAR |KEYWORD
salary |INTEGER |INTEGER
salary |INTEGER |INTEGER
;
showAlias

View File

@ -236,6 +236,9 @@ dep.dep_name.keyword|VARCHAR |KEYWORD
dep.from_date |TIMESTAMP |DATE
dep.to_date |TIMESTAMP |DATE
emp_no |INTEGER |INTEGER
extra |STRUCT |OBJECT
extra.info |STRUCT |OBJECT
extra.info.gender |VARCHAR |KEYWORD
extra_gender |VARCHAR |KEYWORD
extra_no |INTEGER |INTEGER
first_name |VARCHAR |TEXT
@ -261,6 +264,9 @@ dep.dep_name.keyword|VARCHAR |KEYWORD
dep.from_date |TIMESTAMP |DATE
dep.to_date |TIMESTAMP |DATE
emp_no |INTEGER |INTEGER
extra |STRUCT |OBJECT
extra.info |STRUCT |OBJECT
extra.info.gender |VARCHAR |KEYWORD
extra_gender |VARCHAR |KEYWORD
extra_no |INTEGER |INTEGER
first_name |VARCHAR |TEXT
@ -270,7 +276,7 @@ hire_date |TIMESTAMP |DATE
languages |TINYINT |BYTE
last_name |VARCHAR |TEXT
last_name.keyword |VARCHAR |KEYWORD
salary |INTEGER |INTEGER
salary |INTEGER |INTEGER
;
describeSimpleIdentifier

View File

@ -0,0 +1,129 @@
//
// Tests testing field alias (introduced in ES 6.4)
//
// filtering
filterEquals
SELECT extra.info.gender gender FROM "test_emp_copy" WHERE gender = 'M' LIMIT 5;
gender
---------------
M
M
M
M
M
;
filterNotEquals
SELECT extra.info.gender gender FROM "test_emp_copy" WHERE gender <> 'M' ORDER BY gender LIMIT 5;
gender
---------------
F
F
F
F
F
;
aggWithNullFilter
SELECT COUNT(*) count FROM test_emp_copy WHERE extra.info.gender IS NULL;
count:l
---------------
10
;
functionOverAlias
SELECT BIT_LENGTH(extra.info.gender) bit FROM test_emp_copy ORDER BY extra.info.gender LIMIT 1;
bit
---------------
8
;
singlePercentileWithoutComma
SELECT extra.info.gender AS gender, PERCENTILE(emp_no, 97) p1 FROM test_emp_copy GROUP BY extra.info.gender;
gender:s | p1:d
null |10019.0
F |10099.51
M |10095.789999999999
;
singlePercentileWithComma
SELECT extra.info.gender AS gender, PERCENTILE(emp_no, 97.76) p1 FROM test_emp_copy GROUP BY extra.info.gender;
gender:s | p1:d
null |10019.0
F |10099.7608
M |10096.2232
;
multiplePercentilesOneWithCommaOneWithout
SELECT extra.info.gender AS gender, PERCENTILE(emp_no, 92.45) p1, PERCENTILE(emp_no, 91) p2 FROM test_emp_copy GROUP BY extra.info.gender;
gender:s | p1:d | p2:d
null |10018.745 |10018.599999999999
F |10098.0085 |10096.119999999999
M |10091.393 |10090.37
;
multiplePercentilesWithoutComma
SELECT extra.info.gender AS gender, PERCENTILE(emp_no, 91) p1, PERCENTILE(emp_no, 89) p2 FROM test_emp_copy GROUP BY extra.info.gender;
gender:s | p1:d | p2:d
null |10018.599999999999 |10018.4
F |10096.119999999999 |10093.74
M |10090.37 |10086.92
;
multiplePercentilesWithComma
SELECT extra.info.gender AS gender, PERCENTILE(emp_no, 85.7) p1, PERCENTILE(emp_no, 94.3) p2 FROM test_emp_copy GROUP BY extra.info.gender;
gender:s | p1:d | p2:d
null |10018.070000000002 |10018.929999999998
F |10091.343 |10098.619
M |10084.349 |10093.502
;
percentileRank
SELECT extra.info.gender AS gender, PERCENTILE_RANK(emp_no, 10025) rank FROM test_emp_copy GROUP BY extra.info.gender;
gender:s | rank:d
null |100.0
F |17.424242424242426
M |15.350877192982457
;
multiplePercentileRanks
SELECT extra.info.gender AS gender, PERCENTILE_RANK(emp_no, 10030.0) rank1, PERCENTILE_RANK(emp_no, 10025) rank2 FROM test_emp_copy GROUP BY extra.info.gender;
gender:s | rank1:d | rank2:d
null |100.0 |100.0
F |21.445221445221442 |17.424242424242426
M |21.929824561403507 |15.350877192982457
;
multiplePercentilesAndPercentileRank
SELECT extra.info.gender AS gender, PERCENTILE(emp_no, 97.76) p1, PERCENTILE(emp_no, 93.3) p2, PERCENTILE_RANK(emp_no, 10025) rank FROM test_emp_copy GROUP BY extra.info.gender;
gender:s | p1:d | p2:d | rank:d
null |10019.0 |10018.83 |100.0
F |10099.7608 |10098.289 |17.424242424242426
M |10096.2232 |10092.362 |15.350877192982457
;
kurtosisAndSkewnessGroup
SELECT extra.info.gender AS gender, KURTOSIS(salary) k, SKEWNESS(salary) s FROM test_emp_copy GROUP BY extra.info.gender;
gender:s | k:d | s:d
null |2.2215791166941923 |-0.03373126000214023
F |1.7873117044424276 |0.05504995122217512
M |2.280646181070106 |0.44302407229580243
;

View File

@ -26,7 +26,6 @@ import org.elasticsearch.cluster.metadata.MappingMetaData;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.DateEsField;
import org.elasticsearch.xpack.sql.type.EsField;
@ -351,12 +350,18 @@ public class IndexResolver {
EsField parent = flattedMapping.get(parentName);
if (parent == null) {
Map<String, FieldCapabilities> map = globalCaps.get(parentName);
Function<String, EsField> fieldFunction;
// lack of parent implies the field is an alias
if (map == null) {
throw new SqlIllegalArgumentException("Cannot find field {}; this is likely a bug", parentName);
// as such, create the field manually
fieldFunction = s -> createField(s, DataType.OBJECT.name(), new TreeMap<>(), false);
} else {
FieldCapabilities parentCap = map.values().iterator().next();
fieldFunction = s -> createField(s, parentCap.getType(), new TreeMap<>(), parentCap.isAggregatable());
}
FieldCapabilities parentCap = map.values().iterator().next();
parent = createField(parentName, globalCaps, hierarchicalMapping, flattedMapping,
s -> createField(s, parentCap.getType(), new TreeMap<>(), parentCap.isAggregatable()));
parent = createField(parentName, globalCaps, hierarchicalMapping, flattedMapping, fieldFunction);
}
parentProps = parent.getProperties();
}
@ -368,7 +373,7 @@ public class IndexResolver {
return esField;
}
private static EsField createField(String fieldName, String typeName, Map<String, EsField> props, boolean isAggregateable) {
DataType esType = DataType.fromTypeName(typeName);
switch (esType) {

View File

@ -8,8 +8,8 @@ package org.elasticsearch.xpack.sql.plan.logical;
import org.elasticsearch.xpack.sql.analysis.index.EsIndex;
import org.elasticsearch.xpack.sql.expression.Attribute;
import org.elasticsearch.xpack.sql.expression.FieldAttribute;
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.type.EsField;
import java.util.ArrayList;

View File

@ -12,8 +12,8 @@ import org.elasticsearch.xpack.sql.expression.predicate.regex.LikePattern;
import org.elasticsearch.xpack.sql.session.Rows;
import org.elasticsearch.xpack.sql.session.SchemaRowSet;
import org.elasticsearch.xpack.sql.session.SqlSession;
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.elasticsearch.xpack.sql.tree.Source;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.EsField;
import org.elasticsearch.xpack.sql.type.KeywordEsField;
@ -80,6 +80,7 @@ public class ShowColumns extends Command {
DataType dt = field.getDataType();
String name = e.getKey();
if (dt != null) {
// show only fields that exist in ES
rows.add(asList(prefix != null ? prefix + "." + name : name, dt.sqlName(), dt.name()));
if (field.getProperties().isEmpty() == false) {
String newPrefix = prefix != null ? prefix + "." + name : name;

View File

@ -99,7 +99,7 @@ public class EsField {
return false;
}
EsField field = (EsField) o;
return aggregatable == field.aggregatable && esDataType == field.esDataType
return aggregatable == field.aggregatable && esDataType == field.esDataType
&& Objects.equals(name, field.name)
&& Objects.equals(properties, field.properties);
}

View File

@ -21,11 +21,16 @@ import org.elasticsearch.xpack.sql.expression.predicate.conditional.NullIf;
import org.elasticsearch.xpack.sql.parser.SqlParser;
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.sql.stats.Metrics;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.EsField;
import org.elasticsearch.xpack.sql.type.TypesTests;
import java.util.LinkedHashMap;
import java.util.Map;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonMap;
public class VerifierErrorMessagesTests extends ESTestCase {
private SqlParser parser = new SqlParser();
@ -97,7 +102,27 @@ public class VerifierErrorMessagesTests extends ESTestCase {
public void testColumnWithNoSubFields() {
assertEquals("1:8: Cannot determine columns for [text.*]", error("SELECT text.* FROM test"));
}
public void testFieldAliasTypeWithoutHierarchy() {
Map<String, EsField> mapping = new LinkedHashMap<>();
mapping.put("field", new EsField("field", DataType.OBJECT,
singletonMap("alias", new EsField("alias", DataType.KEYWORD, emptyMap(), true)), false));
IndexResolution resolution = IndexResolution.valid(new EsIndex("test", mapping));
// check the nested alias is seen
accept(resolution, "SELECT field.alias FROM test");
// or its hierarhcy
accept(resolution, "SELECT field.* FROM test");
// check typos
assertEquals("1:8: Unknown column [field.alas], did you mean [field.alias]?", error(resolution, "SELECT field.alas FROM test"));
// non-existing parents for aliases are not seen by the user
assertEquals("1:8: Cannot use field [field] type [object] only its subfields", error(resolution, "SELECT field FROM test"));
}
public void testMultipleColumnsWithWildcard1() {
assertEquals("1:14: Unknown column [a]\n" +
"line 1:17: Unknown column [b]\n" +