Skip to content

Commit 46cca7f

Browse files
committed
SQL: Fix esType for DATETIME/DATE and INTERVALS (#38179)
Since introduction of data types that don't have a corresponding type in ES the `esType` is error-prone when used for `unmappedType()` calls. Moreover since the renaming of `DATE` to `DATETIME` and the introduction of an actual date-only `DATE` the `esType` would return `datetime` which is not a valid type for ES mapping. Fixes: #38051
1 parent 571cf52 commit 46cca7f

File tree

22 files changed

+86
-80
lines changed

22 files changed

+86
-80
lines changed

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ else if (DataTypes.isUnsupported(fa.dataType())) {
219219
// compound fields
220220
else if (allowCompound == false && fa.dataType().isPrimitive() == false) {
221221
named = u.withUnresolvedMessage(
222-
"Cannot use field [" + fa.name() + "] type [" + fa.dataType().esType + "] only its subfields");
222+
"Cannot use field [" + fa.name() + "] type [" + fa.dataType().typeName + "] only its subfields");
223223
}
224224
}
225225
return named;
@@ -1228,4 +1228,4 @@ protected boolean skipResolved() {
12281228
return true;
12291229
}
12301230
}
1231-
}
1231+
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -682,7 +682,7 @@ private static void validateInExpression(LogicalPlan p, Set<Failure> localFailur
682682
for (Expression value : in.list()) {
683683
if (areTypesCompatible(dt, value.dataType()) == false) {
684684
localFailures.add(fail(value, "expected data type [{}], value provided is of type [{}]",
685-
dt.esType, value.dataType().esType));
685+
dt.typeName, value.dataType().typeName));
686686
return;
687687
}
688688
}
@@ -703,7 +703,7 @@ private static void validateConditional(LogicalPlan p, Set<Failure> localFailure
703703
} else {
704704
if (areTypesCompatible(dt, child.dataType()) == false) {
705705
localFailures.add(fail(child, "expected data type [{}], value provided is of type [{}]",
706-
dt.esType, child.dataType().esType));
706+
dt.typeName, child.dataType().typeName));
707707
return;
708708
}
709709
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/SourceGenerator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,4 +175,4 @@ private static void disableSource(SearchSourceBuilder builder) {
175175
builder.storedFields(NO_STORED_FIELD);
176176
}
177177
}
178-
}
178+
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public String getWriteableName() {
8787
@Override
8888
public void writeTo(StreamOutput out) throws IOException {
8989
out.writeString(fieldName);
90-
out.writeOptionalString(dataType == null ? null : dataType.esType);
90+
out.writeOptionalString(dataType == null ? null : dataType.typeName);
9191
out.writeBoolean(useDocValue);
9292
out.writeOptionalString(hitName);
9393
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ public static TypeResolution typeMustBe(Expression e,
191191
paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? "" : " " + paramOrd.name().toLowerCase(Locale.ROOT),
192192
acceptedTypesForErrorMsg(acceptedTypes),
193193
Expressions.name(e),
194-
e.dataType().esType));
194+
e.dataType().typeName));
195195
}
196196

197197
private static String acceptedTypesForErrorMsg(String... acceptedTypes) {

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/TopHits.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ protected TypeResolution resolveType() {
4646
((FieldAttribute) field()).exactAttribute();
4747
} catch (MappingException ex) {
4848
return new TypeResolution(format(null, "[{}] cannot operate on first argument field of data type [{}]",
49-
functionName(), field().dataType().esType));
49+
functionName(), field().dataType().typeName));
5050
}
5151

5252
if (orderField() != null) {
@@ -59,7 +59,7 @@ protected TypeResolution resolveType() {
5959
((FieldAttribute) orderField()).exactAttribute();
6060
} catch (MappingException ex) {
6161
return new TypeResolution(format(null, "[{}] cannot operate on second argument field of data type [{}]",
62-
functionName(), orderField().dataType().esType));
62+
functionName(), orderField().dataType().typeName));
6363
}
6464
}
6565
return TypeResolution.TYPE_RESOLVED;

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Sub.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77

88
import org.elasticsearch.xpack.sql.expression.Expression;
99
import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.BinaryArithmeticProcessor.BinaryArithmeticOperation;
10-
import org.elasticsearch.xpack.sql.tree.Source;
1110
import org.elasticsearch.xpack.sql.tree.NodeInfo;
11+
import org.elasticsearch.xpack.sql.tree.Source;
1212
import org.elasticsearch.xpack.sql.type.DataTypes;
1313

1414
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
@@ -36,7 +36,7 @@ protected Sub replaceChildren(Expression newLeft, Expression newRight) {
3636
protected TypeResolution resolveWithIntervals() {
3737
if (right().dataType().isDateBased() && DataTypes.isInterval(left().dataType())) {
3838
return new TypeResolution(format(null, "Cannot subtract a {}[{}] from an interval[{}]; do you mean the reverse?",
39-
right().dataType().esType, right().source().text(), left().source().text()));
39+
right().dataType().typeName, right().source().text(), left().source().text()));
4040
}
4141
return TypeResolution.TYPE_RESOLVED;
4242
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/ShowColumns.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ private void fillInRows(Map<String, EsField> mapping, String prefix, List<List<?
8181
String name = e.getKey();
8282
if (dt != null) {
8383
// show only fields that exist in ES
84-
rows.add(asList(prefix != null ? prefix + "." + name : name, dt.sqlName(), dt.esType));
84+
rows.add(asList(prefix != null ? prefix + "." + name : name, dt.sqlName(), dt.typeName));
8585
if (field.getProperties().isEmpty() == false) {
8686
String newPrefix = prefix != null ? prefix + "." + name : name;
8787
fillInRows(field.getProperties(), newPrefix, rows);

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumns.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import java.sql.DatabaseMetaData;
2525
import java.util.ArrayList;
2626
import java.util.List;
27-
import java.util.Locale;
2827
import java.util.Map;
2928
import java.util.Objects;
3029
import java.util.regex.Pattern;
@@ -143,7 +142,7 @@ static void fillInRows(String clusterName, String indexName, Map<String, EsField
143142
indexName,
144143
name,
145144
odbcCompatible(type.sqlType.getVendorTypeNumber(), isOdbcClient),
146-
type.esType.toUpperCase(Locale.ROOT),
145+
type.toString(),
147146
type.displaySize,
148147
// TODO: is the buffer_length correct?
149148
type.size,
@@ -208,4 +207,4 @@ public boolean equals(Object obj) {
208207
&& Objects.equals(pattern, other.pattern)
209208
&& Objects.equals(columnPattern, other.columnPattern);
210209
}
211-
}
210+
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTypes.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,14 @@
1111
import org.elasticsearch.xpack.sql.session.Rows;
1212
import org.elasticsearch.xpack.sql.session.SchemaRowSet;
1313
import org.elasticsearch.xpack.sql.session.SqlSession;
14-
import org.elasticsearch.xpack.sql.tree.Source;
1514
import org.elasticsearch.xpack.sql.tree.NodeInfo;
15+
import org.elasticsearch.xpack.sql.tree.Source;
1616
import org.elasticsearch.xpack.sql.type.DataType;
1717
import org.elasticsearch.xpack.sql.type.DataTypes;
1818

1919
import java.sql.DatabaseMetaData;
2020
import java.util.Comparator;
2121
import java.util.List;
22-
import java.util.Locale;
2322
import java.util.stream.Stream;
2423

2524
import static java.util.Arrays.asList;
@@ -81,7 +80,7 @@ public final void execute(SqlSession session, ActionListener<SchemaRowSet> liste
8180
List<List<?>> rows = values
8281
// sort by SQL int type (that's what the JDBC/ODBC specs want) followed by name
8382
.sorted(Comparator.comparing((DataType t) -> t.sqlType.getVendorTypeNumber()).thenComparing(DataType::sqlName))
84-
.map(t -> asList(t.esType.toUpperCase(Locale.ROOT),
83+
.map(t -> asList(t.toString(),
8584
t.sqlType.getVendorTypeNumber(),
8685
//https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/column-size?view=sql-server-2017
8786
t.defaultPrecision,
@@ -132,4 +131,4 @@ public boolean equals(Object obj) {
132131

133132
return type.equals(((SysTypes) obj).type);
134133
}
135-
}
134+
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plugin/TransportSqlQueryAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ static SqlQueryResponse createResponse(SqlQueryRequest request, SchemaRowSet row
8888
List<ColumnInfo> columns = new ArrayList<>(rowSet.columnCount());
8989
for (Schema.Entry entry : rowSet.schema()) {
9090
if (Mode.isDriver(request.mode())) {
91-
columns.add(new ColumnInfo("", entry.name(), entry.type().esType, entry.type().displaySize));
91+
columns.add(new ColumnInfo("", entry.name(), entry.type().typeName, entry.type().displaySize));
9292
} else {
93-
columns.add(new ColumnInfo("", entry.name(), entry.type().esType));
93+
columns.add(new ColumnInfo("", entry.name(), entry.type().typeName));
9494
}
9595
}
9696
columns = unmodifiableList(columns);

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/TopHitsAggRef.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,6 @@ public DataType fieldDataType() {
3333

3434
@Override
3535
public String toString() {
36-
return ">" + name + "[" + fieldDataType.esType + "]";
36+
return ">" + name + "[" + fieldDataType.typeName + "]";
3737
}
3838
}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataType.java

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,41 +23,41 @@
2323
public enum DataType {
2424

2525
// @formatter:off
26-
// jdbc type, size, defPrecision,dispSize, int, rat, docvals
27-
NULL( JDBCType.NULL, 0, 0, 0, false, false, false),
28-
UNSUPPORTED( JDBCType.OTHER, 0, 0, 0, false, false, false),
29-
BOOLEAN( JDBCType.BOOLEAN, 1, 1, 1, false, false, false),
30-
BYTE( JDBCType.TINYINT, Byte.BYTES, 3, 5, true, false, true),
31-
SHORT( JDBCType.SMALLINT, Short.BYTES, 5, 6, true, false, true),
32-
INTEGER( JDBCType.INTEGER, Integer.BYTES, 10, 11, true, false, true),
33-
LONG( JDBCType.BIGINT, Long.BYTES, 19, 20, true, false, true),
26+
// esType jdbc type, size, defPrecision,dispSize, int, rat, docvals
27+
NULL( "null", JDBCType.NULL, 0, 0, 0, false, false, false),
28+
UNSUPPORTED( JDBCType.OTHER, 0, 0, 0, false, false, false),
29+
BOOLEAN( "boolean", JDBCType.BOOLEAN, 1, 1, 1, false, false, false),
30+
BYTE( "byte", JDBCType.TINYINT, Byte.BYTES, 3, 5, true, false, true),
31+
SHORT( "short", JDBCType.SMALLINT, Short.BYTES, 5, 6, true, false, true),
32+
INTEGER( "integer", JDBCType.INTEGER, Integer.BYTES, 10, 11, true, false, true),
33+
LONG( "long", JDBCType.BIGINT, Long.BYTES, 19, 20, true, false, true),
3434
// 53 bits defaultPrecision ~ 15(15.95) decimal digits (53log10(2)),
35-
DOUBLE( JDBCType.DOUBLE, Double.BYTES, 15, 25, false, true, true),
35+
DOUBLE( "double", JDBCType.DOUBLE, Double.BYTES, 15, 25, false, true, true),
3636
// 24 bits defaultPrecision - 24*log10(2) =~ 7 (7.22)
37-
FLOAT( JDBCType.REAL, Float.BYTES, 7, 15, false, true, true),
38-
HALF_FLOAT( JDBCType.FLOAT, Double.BYTES, 16, 25, false, true, true),
37+
FLOAT( "float", JDBCType.REAL, Float.BYTES, 7, 15, false, true, true),
38+
HALF_FLOAT( "half_float", JDBCType.FLOAT, Double.BYTES, 16, 25, false, true, true),
3939
// precision is based on long
40-
SCALED_FLOAT( JDBCType.FLOAT, Double.BYTES, 19, 25, false, true, true),
41-
KEYWORD( JDBCType.VARCHAR, Integer.MAX_VALUE, 256, 0, false, false, true),
42-
TEXT( JDBCType.VARCHAR, Integer.MAX_VALUE, Integer.MAX_VALUE, 0, false, false, false),
43-
OBJECT( JDBCType.STRUCT, -1, 0, 0, false, false, false),
44-
NESTED( JDBCType.STRUCT, -1, 0, 0, false, false, false),
45-
BINARY( JDBCType.VARBINARY, -1, Integer.MAX_VALUE, 0, false, false, false),
46-
DATE( JDBCType.DATE, Long.BYTES, 10, 10, false, false, true),
40+
SCALED_FLOAT( "scaled_float", JDBCType.FLOAT, Double.BYTES, 19, 25, false, true, true),
41+
KEYWORD( "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE, 256, 0, false, false, true),
42+
TEXT( "text", JDBCType.VARCHAR, Integer.MAX_VALUE, Integer.MAX_VALUE, 0, false, false, false),
43+
OBJECT( "object", JDBCType.STRUCT, -1, 0, 0, false, false, false),
44+
NESTED( "nested", JDBCType.STRUCT, -1, 0, 0, false, false, false),
45+
BINARY( "binary", JDBCType.VARBINARY, -1, Integer.MAX_VALUE, 0, false, false, false),
46+
DATE( JDBCType.DATE, Long.BYTES, 10, 10, false, false, true),
4747
// since ODBC and JDBC interpret precision for Date as display size
4848
// the precision is 23 (number of chars in ISO8601 with millis) + Z (the UTC timezone)
4949
// see https://github.com/elastic/elasticsearch/issues/30386#issuecomment-386807288
50-
DATETIME( JDBCType.TIMESTAMP, Long.BYTES, 24, 24, false, false, true),
50+
DATETIME( "date", JDBCType.TIMESTAMP, Long.BYTES, 24, 24, false, false, true),
5151
//
5252
// specialized types
5353
//
5454
// IP can be v4 or v6. The latter has 2^128 addresses or 340,282,366,920,938,463,463,374,607,431,768,211,456
5555
// aka 39 chars
56-
IP( JDBCType.VARCHAR, 39, 39, 0, false, false, true),
56+
IP( "ip", JDBCType.VARCHAR, 39, 39, 0, false, false, true),
5757
//
5858
// INTERVALS
5959
// the list is long as there are a lot of variations and that's what clients (ODBC) expect
60-
// jdbc type, size, prec,disp, int, rat, docvals
60+
// esType:null jdbc type, size, prec,disp, int, rat, docvals
6161
INTERVAL_YEAR( ExtTypes.INTERVAL_YEAR, Integer.BYTES, 7, 7, false, false, false),
6262
INTERVAL_MONTH( ExtTypes.INTERVAL_MONTH, Integer.BYTES, 7, 7, false, false, false),
6363
INTERVAL_DAY( ExtTypes.INTERVAL_DAY, Long.BYTES, 23, 23, false, false, false),
@@ -126,7 +126,12 @@ public enum DataType {
126126

127127

128128
/**
129-
* Elasticsearch type name
129+
* Type's name used for error messages and column info for the clients
130+
*/
131+
public final String typeName;
132+
133+
/**
134+
* Elasticsearch data type that it maps to
130135
*/
131136
public final String esType;
132137

@@ -176,7 +181,13 @@ public enum DataType {
176181

177182
DataType(SQLType sqlType, int size, int defaultPrecision, int displaySize, boolean isInteger,
178183
boolean isRational, boolean defaultDocValues) {
179-
this.esType = name().toLowerCase(Locale.ROOT);
184+
this(null, sqlType, size, defaultPrecision, displaySize, isInteger, isRational, defaultDocValues);
185+
}
186+
187+
DataType(String esType, SQLType sqlType, int size, int defaultPrecision, int displaySize, boolean isInteger,
188+
boolean isRational, boolean defaultDocValues) {
189+
this.typeName = name().toLowerCase(Locale.ROOT);
190+
this.esType = esType;
180191
this.sqlType = sqlType;
181192
this.size = size;
182193
this.defaultPrecision = defaultPrecision;
@@ -228,8 +239,6 @@ public static DataType fromOdbcType(String odbcType) {
228239

229240
/**
230241
* Creates returns DataType enum corresponding to the specified es type
231-
* <p>
232-
* For any dataType DataType.fromTypeName(dataType.esType) == dataType
233242
*/
234243
public static DataType fromTypeName(String esType) {
235244
String uppercase = esType.toUpperCase(Locale.ROOT);

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/Schema.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,9 @@ public String toString() {
120120
}
121121
sb.append(names.get(i));
122122
sb.append(":");
123-
sb.append(types.get(i).esType);
123+
sb.append(types.get(i).typeName);
124124
}
125125
sb.append("]");
126126
return sb.toString();
127127
}
128-
}
128+
}

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/index/IndexResolverTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public static Map<String, Map<String, FieldCapabilities>> fromMappings(EsIndex..
154154
if (entry.getValue().size() > 1) {
155155
for (EsIndex index : indices) {
156156
EsField field = index.mapping().get(fieldName);
157-
UpdateableFieldCapabilities fieldCaps = (UpdateableFieldCapabilities) caps.get(field.getDataType().esType);
157+
UpdateableFieldCapabilities fieldCaps = (UpdateableFieldCapabilities) caps.get(field.getDataType().typeName);
158158
fieldCaps.indices.add(index.name());
159159
}
160160
//TODO: what about nonAgg/SearchIndices?
@@ -171,7 +171,7 @@ private static void addFieldCaps(String parent, EsField field, String indexName,
171171
map = new HashMap<>();
172172
merged.put(fieldName, map);
173173
}
174-
FieldCapabilities caps = map.computeIfAbsent(field.getDataType().esType,
174+
FieldCapabilities caps = map.computeIfAbsent(field.getDataType().typeName,
175175
esType -> new UpdateableFieldCapabilities(fieldName, esType,
176176
isSearchable(field.getDataType()),
177177
isAggregatable(field.getDataType())));

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/ParameterTests.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public class ParameterTests extends ESTestCase {
2828
public void testSingleParameter() {
2929
Expression expression = new SqlParser().createExpression("a = \n?",
3030
Collections.singletonList(
31-
new SqlTypedParamValue(DataType.KEYWORD.esType, "foo")
31+
new SqlTypedParamValue(DataType.KEYWORD.typeName, "foo")
3232
));
3333
logger.info(expression);
3434
assertThat(expression, instanceOf(Equals.class));
@@ -42,10 +42,10 @@ public void testSingleParameter() {
4242

4343
public void testMultipleParameters() {
4444
Expression expression = new SqlParser().createExpression("(? + ? * ?) - ?", Arrays.asList(
45-
new SqlTypedParamValue(DataType.LONG.esType, 1L),
46-
new SqlTypedParamValue(DataType.LONG.esType, 2L),
47-
new SqlTypedParamValue(DataType.LONG.esType, 3L),
48-
new SqlTypedParamValue(DataType.LONG.esType, 4L)
45+
new SqlTypedParamValue(DataType.LONG.typeName, 1L),
46+
new SqlTypedParamValue(DataType.LONG.typeName, 2L),
47+
new SqlTypedParamValue(DataType.LONG.typeName, 3L),
48+
new SqlTypedParamValue(DataType.LONG.typeName, 4L)
4949
));
5050
assertThat(expression, instanceOf(Sub.class));
5151
Sub sub = (Sub) expression;
@@ -62,9 +62,9 @@ public void testMultipleParameters() {
6262
public void testNotEnoughParameters() {
6363
ParsingException ex = expectThrows(ParsingException.class,
6464
() -> new SqlParser().createExpression("(? + ? * ?) - ?", Arrays.asList(
65-
new SqlTypedParamValue(DataType.LONG.esType, 1L),
66-
new SqlTypedParamValue(DataType.LONG.esType, 2L),
67-
new SqlTypedParamValue(DataType.LONG.esType, 3L)
65+
new SqlTypedParamValue(DataType.LONG.typeName, 1L),
66+
new SqlTypedParamValue(DataType.LONG.typeName, 2L),
67+
new SqlTypedParamValue(DataType.LONG.typeName, 3L)
6868
)));
6969
assertThat(ex.getMessage(), containsString("Not enough actual parameters"));
7070
}

0 commit comments

Comments
 (0)