honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SQL: Improve correctness of SYS COLUMNS & TYPES (#30418) 

Tweak the return data, in particular with regards for ODBC columns to
better align with the spec
Fix order for SYS TYPES and TABLES according to the JDBC/ODBC spec

Fix #30386
Fix #30521
This commit is contained in:
Costin Leau 2018-05-11 10:17:01 +03:00 committed by GitHub
parent 73b08d937b
commit 2594c1fb38
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 175 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/type/DataType.java
View File

@ -25,8 +25,8 @@ public enum DataType {
SHORT( JDBCType.SMALLINT, Short.class, Short.BYTES, 5, 6, true, false, true),
INTEGER( JDBCType.INTEGER, Integer.class, Integer.BYTES, 10, 11, true, false, true),
LONG( JDBCType.BIGINT, Long.class, Long.BYTES, 19, 20, true, false, true),
// 53 bits defaultPrecision ~ 16(15.95) decimal digits (53log10(2)),
DOUBLE( JDBCType.DOUBLE, Double.class, Double.BYTES, 16, 25, false, true, true),
// 53 bits defaultPrecision ~ 15(15.95) decimal digits (53log10(2)),
DOUBLE( JDBCType.DOUBLE, Double.class, Double.BYTES, 15, 25, false, true, true),
// 24 bits defaultPrecision - 24*log10(2) =~ 7 (7.22)
FLOAT( JDBCType.REAL, Float.class, Float.BYTES, 7, 15, false, true, true),
HALF_FLOAT( JDBCType.FLOAT, Double.class, Double.BYTES, 16, 25, false, true, true),
@ -37,7 +37,10 @@ public enum DataType {
OBJECT( JDBCType.STRUCT, null, -1, 0, 0),
NESTED( JDBCType.STRUCT, null, -1, 0, 0),
BINARY( JDBCType.VARBINARY, byte[].class, -1, Integer.MAX_VALUE, 0),
DATE( JDBCType.TIMESTAMP, Timestamp.class, Long.BYTES, 19, 20);
// since ODBC and JDBC interpret precision for Date as display size,
// the precision is 23 (number of chars in ISO8601 with millis) + Z (the UTC timezone)
// see https://github.com/elastic/elasticsearch/issues/30386#issuecomment-386807288
DATE( JDBCType.TIMESTAMP, Timestamp.class, Long.BYTES, 24, 24);
// @formatter:on
private static final Map<JDBCType, DataType> jdbcToEs;
@ -75,7 +78,7 @@ public enum DataType {
* <p>
* Specified column size. For numeric data, this is the maximum precision. For character
* data, this is the length in characters. For datetime datatypes, this is the length in characters of the
* String representation (assuming the maximum allowed defaultPrecision of the fractional seconds component).
* String representation (assuming the maximum allowed defaultPrecision of the fractional milliseconds component).
*/
public final int defaultPrecision;

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

@ -17,6 +17,7 @@ import org.elasticsearch.xpack.sql.session.SqlSession;
import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.DataTypes;
import org.elasticsearch.xpack.sql.type.EsField;
import java.sql.DatabaseMetaData;
@ -29,7 +30,6 @@ import java.util.regex.Pattern;
import static java.util.Arrays.asList;
import static org.elasticsearch.xpack.sql.type.DataType.INTEGER;
import static org.elasticsearch.xpack.sql.type.DataType.NULL;
import static org.elasticsearch.xpack.sql.type.DataType.SHORT;
/**
@ -133,11 +133,7 @@ public class SysColumns extends Command {
type.size,
// no DECIMAL support
null,
// RADIX - Determines how numbers returned by COLUMN_SIZE and DECIMAL_DIGITS should be interpreted.
// 10 means they represent the number of decimal digits allowed for the column.
// 2 means they represent the number of bits allowed for the column.
// null means radix is not applicable for the given type.
type.isInteger ? Integer.valueOf(10) : type.isRational ? Integer.valueOf(2) : null,
DataTypes.metaSqlRadix(type),
// everything is nullable
DatabaseMetaData.columnNullable,
// no remarks
@ -145,9 +141,9 @@ public class SysColumns extends Command {
// no column def
null,
// SQL_DATA_TYPE apparently needs to be same as DATA_TYPE except for datetime and interval data types
type.jdbcType.getVendorTypeNumber(),
DataTypes.metaSqlDataType(type),
// SQL_DATETIME_SUB ?
null,
DataTypes.metaSqlDateTimeSub(type),
// char octet length
type.isString() || type == DataType.BINARY ? type.size : null,
// position

3
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTableTypes.java
View File

@ -15,6 +15,7 @@ import org.elasticsearch.xpack.sql.session.SqlSession;
import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import java.util.Comparator;
import java.util.List;
import static java.util.Collections.singletonList;
@ -43,6 +44,8 @@ public class SysTableTypes extends Command {
@Override
public final void execute(SqlSession session, ActionListener<SchemaRowSet> listener) {
listener.onResponse(Rows.of(output(), IndexType.VALID.stream()
// *DBC requires ascending order
.sorted(Comparator.comparing(t -> t.toSql()))
.map(t -> singletonList(t.toSql()))
.collect(toList())));
}

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

@ -14,6 +14,7 @@ import org.elasticsearch.xpack.sql.session.SqlSession;
import org.elasticsearch.xpack.sql.tree.Location;
import org.elasticsearch.xpack.sql.tree.NodeInfo;
import org.elasticsearch.xpack.sql.type.DataType;
import org.elasticsearch.xpack.sql.type.DataTypes;
import java.sql.DatabaseMetaData;
import java.util.Comparator;
@ -67,9 +68,10 @@ public class SysTypes extends Command {
public final void execute(SqlSession session, ActionListener<SchemaRowSet> listener) {
List<List<?>> rows = Stream.of(DataType.values())
// sort by SQL int type (that's what the JDBC/ODBC specs want)
.sorted(Comparator.comparing(t -> t.jdbcType))
.sorted(Comparator.comparing(t -> t.jdbcType.getVendorTypeNumber()))
.map(t -> asList(t.esType.toUpperCase(Locale.ROOT),
t.jdbcType.getVendorTypeNumber(),
//https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/column-size?view=sql-server-2017
t.defaultPrecision,
"'",
"'",
@ -83,16 +85,17 @@ public class SysTypes extends Command {
// only numerics are signed
!t.isSigned(),
//no fixed precision scale SQL_FALSE
false,
null,
null,
null,
Boolean.FALSE,
// not auto-incremented
Boolean.FALSE,
null,
DataTypes.metaSqlMinimumScale(t),
DataTypes.metaSqlMaximumScale(t),
// SQL_DATA_TYPE - ODBC wants this to be not null
0,
null,
DataTypes.metaSqlDataType(t),
DataTypes.metaSqlDateTimeSub(t),
// Radix
t.isInteger ? Integer.valueOf(10) : (t.isRational ? Integer.valueOf(2) : null),
DataTypes.metaSqlRadix(t),
null
))
.collect(toList());

69
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypes.java
View File

@ -51,4 +51,71 @@ public abstract class DataTypes {
}
throw new SqlIllegalArgumentException("No idea what's the DataType for {}", value.getClass());
}
}
//
// Metadata methods, mainly for ODBC.
// As these are fairly obscure and limited in use, there is no point to promote them as a full type methods
// hence why they appear here as utility methods.
//
// https://docs.microsoft.com/en-us/sql/relational-databases/native-client-odbc-date-time/metadata-catalog
// https://github.com/elastic/elasticsearch/issues/30386
public static Integer metaSqlDataType(DataType t) {
if (t == DataType.DATE) {
// ODBC SQL_DATETME
return Integer.valueOf(9);
}
// this is safe since the vendor SQL types are short despite the return value
return t.jdbcType.getVendorTypeNumber();
}
// https://github.com/elastic/elasticsearch/issues/30386
// https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function?view=sql-server-2017
public static Integer metaSqlDateTimeSub(DataType t) {
if (t == DataType.DATE) {
// ODBC SQL_CODE_TIMESTAMP
return Integer.valueOf(3);
}
// ODBC null
return 0;
}
// https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/decimal-digits?view=sql-server-2017
public static Short metaSqlMinimumScale(DataType t) {
// TODO: return info for HALF/SCALED_FLOATS (should be based on field not type)
if (t == DataType.DATE) {
return Short.valueOf((short) 3);
}
if (t.isInteger) {
return Short.valueOf((short) 0);
}
// minimum scale?
if (t.isRational) {
return Short.valueOf((short) 0);
}
return null;
}
public static Short metaSqlMaximumScale(DataType t) {
// TODO: return info for HALF/SCALED_FLOATS (should be based on field not type)
if (t == DataType.DATE) {
return Short.valueOf((short) 3);
}
if (t.isInteger) {
return Short.valueOf((short) 0);
}
if (t.isRational) {
return Short.valueOf((short) t.defaultPrecision);
}
return null;
}
// https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function?view=sql-server-2017
public static Integer metaSqlRadix(DataType t) {
// RADIX - Determines how numbers returned by COLUMN_SIZE and DECIMAL_DIGITS should be interpreted.
// 10 means they represent the number of decimal digits allowed for the column.
// 2 means they represent the number of bits allowed for the column.
// null means radix is not applicable for the given type.
return t.isInteger ? Integer.valueOf(10) : (t.isRational ? Integer.valueOf(2) : null);
}
}

7
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DateEsField.java
View File

@ -25,13 +25,6 @@ public class DateEsField extends EsField {
this.formats = CollectionUtils.isEmpty(formats) ? DEFAULT_FORMAT : Arrays.asList(formats);
}
@Override
public int getPrecision() {
// same as Long
// TODO: based this on format string
return 19;
}
public List<String> getFormats() {
return formats;
}

11
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java
View File

@ -38,6 +38,13 @@ public class SysColumnsTests extends ESTestCase {
assertEquals(null, radix(row));
assertEquals(Integer.MAX_VALUE, bufferLength(row));
row = rows.get(4);
assertEquals("date", name(row));
assertEquals(Types.TIMESTAMP, sqlType(row));
assertEquals(null, radix(row));
assertEquals(24, precision(row));
assertEquals(8, bufferLength(row));
row = rows.get(7);
assertEquals("some.dotted", name(row));
assertEquals(Types.STRUCT, sqlType(row));
@ -59,6 +66,10 @@ public class SysColumnsTests extends ESTestCase {
return list.get(4);
}
private static Object precision(List<?> list) {
return list.get(6);
}
private static Object bufferLength(List<?> list) {
return list.get(7);
}

6
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysParserTests.java
View File

@ -57,8 +57,8 @@ public class SysParserTests extends ESTestCase {
public void testSysTypes() throws Exception {
Command cmd = sql("SYS TYPES").v1();
List<String> names = asList("BYTE", "SHORT", "INTEGER", "LONG", "HALF_FLOAT", "SCALED_FLOAT", "FLOAT", "DOUBLE", "KEYWORD", "TEXT",
"DATE", "BINARY", "NULL", "UNSUPPORTED", "OBJECT", "NESTED", "BOOLEAN");
List<String> names = asList("BYTE", "LONG", "BINARY", "NULL", "INTEGER", "SHORT", "HALF_FLOAT", "SCALED_FLOAT", "FLOAT", "DOUBLE",
"KEYWORD", "TEXT", "BOOLEAN", "DATE", "UNSUPPORTED", "OBJECT", "NESTED");
cmd.execute(null, ActionListener.wrap(r -> {
assertEquals(19, r.columnCount());
@ -68,6 +68,8 @@ public class SysParserTests extends ESTestCase {
assertFalse(r.column(9, Boolean.class));
// make sure precision is returned as boolean (not int)
assertFalse(r.column(10, Boolean.class));
// no auto-increment
assertFalse(r.column(11, Boolean.class));
for (int i = 0; i < r.size(); i++) {
assertEquals(names.get(i), r.column(0));

4
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysTableTypesTests.java
View File

@ -41,9 +41,9 @@ public class SysTableTypesTests extends ESTestCase {
sql.v1().execute(sql.v2(), ActionListener.wrap(r -> {
assertEquals(2, r.size());
assertEquals("BASE TABLE", r.column(0));
r.advanceRow();
assertEquals("ALIAS", r.column(0));
r.advanceRow();
assertEquals("BASE TABLE", r.column(0));
}, ex -> fail(ex.getMessage())));
}
}

58
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypesTests.java Normal file
View File

@ -0,0 +1,58 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.sql.type;
import org.elasticsearch.test.ESTestCase;
import static org.elasticsearch.xpack.sql.type.DataType.DATE;
import static org.elasticsearch.xpack.sql.type.DataType.FLOAT;
import static org.elasticsearch.xpack.sql.type.DataType.KEYWORD;
import static org.elasticsearch.xpack.sql.type.DataType.LONG;
import static org.elasticsearch.xpack.sql.type.DataTypes.metaSqlDataType;
import static org.elasticsearch.xpack.sql.type.DataTypes.metaSqlDateTimeSub;
import static org.elasticsearch.xpack.sql.type.DataTypes.metaSqlMaximumScale;
import static org.elasticsearch.xpack.sql.type.DataTypes.metaSqlMinimumScale;
import static org.elasticsearch.xpack.sql.type.DataTypes.metaSqlRadix;
public class DataTypesTests extends ESTestCase {
public void testMetaDataType() {
assertEquals(Integer.valueOf(9), metaSqlDataType(DATE));
DataType t = randomDataTypeNoDate();
assertEquals(t.jdbcType.getVendorTypeNumber(), metaSqlDataType(t));
}
public void testMetaDateTypeSub() {
assertEquals(Integer.valueOf(3), metaSqlDateTimeSub(DATE));
assertEquals(Integer.valueOf(0), metaSqlDateTimeSub(randomDataTypeNoDate()));
}
public void testMetaMinimumScale() {
assertEquals(Short.valueOf((short) 3), metaSqlMinimumScale(DATE));
assertEquals(Short.valueOf((short) 0), metaSqlMinimumScale(LONG));
assertEquals(Short.valueOf((short) 0), metaSqlMinimumScale(FLOAT));
assertNull(metaSqlMinimumScale(KEYWORD));
}
public void testMetaMaximumScale() {
assertEquals(Short.valueOf((short) 3), metaSqlMaximumScale(DATE));
assertEquals(Short.valueOf((short) 0), metaSqlMaximumScale(LONG));
assertEquals(Short.valueOf((short) FLOAT.defaultPrecision), metaSqlMaximumScale(FLOAT));
assertNull(metaSqlMaximumScale(KEYWORD));
}
public void testMetaRadix() {
assertNull(metaSqlRadix(DATE));
assertNull(metaSqlRadix(KEYWORD));
assertEquals(Integer.valueOf(10), metaSqlRadix(LONG));
assertEquals(Integer.valueOf(2), metaSqlRadix(FLOAT));
}
private DataType randomDataTypeNoDate() {
return randomValueOtherThan(DataType.DATE, () -> randomFrom(DataType.values()));
}
}

2
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/TypesTests.java
View File

@ -82,7 +82,7 @@ public class TypesTests extends ESTestCase {
EsField field = mapping.get("date");
assertThat(field.getDataType(), is(DATE));
assertThat(field.hasDocValues(), is(true));
assertThat(field.getPrecision(), is(19));
assertThat(field.getPrecision(), is(24));
DateEsField dfield = (DateEsField) field;
List<String> formats = dfield.getFormats();

12
x-pack/qa/sql/src/main/resources/setup_mock_metadata_get_columns.sql
View File

@ -25,26 +25,26 @@ CREATE TABLE mock (
) AS
SELECT null, 'test1', 'name', 12, 'TEXT', 0, 2147483647, null, null,
1, -- columnNullable
null, null, 12, null, 2147483647, 1, 'YES', null, null, null, null, 'NO', 'NO'
null, null, 12, 0, 2147483647, 1, 'YES', null, null, null, null, 'NO', 'NO'
FROM DUAL
UNION ALL
SELECT null, 'test1', 'name.keyword', 12, 'KEYWORD', 0, 2147483647, null, null,
1, -- columnNullable
null, null, 12, null, 2147483647, 1, 'YES', null, null, null, null, 'NO', 'NO'
null, null, 12, 0, 2147483647, 1, 'YES', null, null, null, null, 'NO', 'NO'
FROM DUAL
UNION ALL
SELECT null, 'test2', 'date', 93, 'DATE', 20, 8, null, null,
SELECT null, 'test2', 'date', 93, 'DATE', 24, 8, null, null,
1, -- columnNullable
null, null, 93, null, null, 1, 'YES', null, null, null, null, 'NO', 'NO'
null, null, 9, 3, null, 1, 'YES', null, null, null, null, 'NO', 'NO'
FROM DUAL
UNION ALL
SELECT null, 'test2', 'float', 7, 'FLOAT', 15, 4, null, 2,
1, -- columnNullable
null, null, 7, null, null, 2, 'YES', null, null, null, null, 'NO', 'NO'
null, null, 7, 0, null, 2, 'YES', null, null, null, null, 'NO', 'NO'
FROM DUAL
UNION ALL
SELECT null, 'test2', 'number', -5, 'LONG', 20, 8, null, 10,
1, -- columnNullable
null, null, -5, null, null, 3, 'YES', null, null, null, null, 'NO', 'NO'
null, null, -5, 0, null, 3, 'YES', null, null, null, null, 'NO', 'NO'
FROM DUAL
;