Skip to content

Commit 1557d77

Browse files
costincostin
authored
SQL: Adjust the precision and scale for drivers (#40467)
Fix #40357
1 parent fed3580 commit 1557d77

File tree

16 files changed

+62
-62
lines changed

16 files changed

+62
-62
lines changed

‎docs/reference/sql/language/data-types.asciidoc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ s|SQL precision
2121
| <<number, `long`>> | long | BIGINT | 19
2222
| <<number, `double`>> | double | DOUBLE | 15
2323
| <<number, `float`>> | float | REAL | 7
24-
| <<number, `half_float`>> | half_float | FLOAT | 16
25-
| <<number, `scaled_float`>> | scaled_float | FLOAT | 19
26-
| <<keyword, `keyword`>> | keyword | VARCHAR | based on <<ignore-above>>
24+
| <<number, `half_float`>> | half_float | FLOAT | 3
25+
| <<number, `scaled_float`>> | scaled_float | DOUBLE | 15
26+
| <<keyword, `keyword`>> | keyword | VARCHAR | 32,766
2727
| <<text, `text`>> | text | VARCHAR | 2,147,483,647
2828
| <<binary, `binary`>> | binary | VARBINARY | 2,147,483,647
2929
| <<date, `date`>> | datetime | TIMESTAMP | 24

‎x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlSecurityIT.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@
2929
import java.util.Map;
3030
import java.util.stream.Collectors;
3131

32+
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.SQL_QUERY_REST_ENDPOINT;
3233
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
3334
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
3435
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
35-
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.SQL_QUERY_REST_ENDPOINT;
3636
import static org.hamcrest.Matchers.containsString;
3737
import static org.hamcrest.Matchers.empty;
3838
import static org.hamcrest.Matchers.equalTo;
@@ -101,9 +101,9 @@ public void expectDescribe(Map<String, List<String>> columns, String user) throw
101101
String mode = randomMode();
102102
Map<String, Object> expected = new HashMap<>(3);
103103
expected.put("columns", Arrays.asList(
104-
columnInfo(mode, "column", "keyword", JDBCType.VARCHAR, 0),
105-
columnInfo(mode, "type", "keyword", JDBCType.VARCHAR, 0),
106-
columnInfo(mode, "mapping", "keyword", JDBCType.VARCHAR, 0)));
104+
columnInfo(mode, "column", "keyword", JDBCType.VARCHAR, 32766),
105+
columnInfo(mode, "type", "keyword", JDBCType.VARCHAR, 32766),
106+
columnInfo(mode, "mapping", "keyword", JDBCType.VARCHAR, 32766)));
107107
List<List<String>> rows = new ArrayList<>(columns.size());
108108
for (Map.Entry<String, List<String>> column : columns.entrySet()) {
109109
List<String> cols = new ArrayList<>();
@@ -120,8 +120,8 @@ public void expectDescribe(Map<String, List<String>> columns, String user) throw
120120
public void expectShowTables(List<String> tables, String user) throws Exception {
121121
String mode = randomMode();
122122
List<Object> columns = new ArrayList<>();
123-
columns.add(columnInfo(mode, "name", "keyword", JDBCType.VARCHAR, 0));
124-
columns.add(columnInfo(mode, "type", "keyword", JDBCType.VARCHAR, 0));
123+
columns.add(columnInfo(mode, "name", "keyword", JDBCType.VARCHAR, 32766));
124+
columns.add(columnInfo(mode, "type", "keyword", JDBCType.VARCHAR, 32766));
125125
Map<String, Object> expected = new HashMap<>();
126126
expected.put("columns", columns);
127127
List<List<String>> rows = new ArrayList<>();

‎x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@
3434
import java.util.List;
3535
import java.util.Map;
3636

37+
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.SQL_QUERY_REST_ENDPOINT;
3738
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
3839
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
3940
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
40-
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.SQL_QUERY_REST_ENDPOINT;
4141

4242
public class UserFunctionIT extends ESRestTestCase {
4343

@@ -81,7 +81,7 @@ public void testSingleRandomUser() throws IOException {
8181

8282
Map<String, Object> expected = new HashMap<>();
8383
expected.put("columns", Arrays.asList(
84-
columnInfo(mode, "USER()", "keyword", JDBCType.VARCHAR, 0)));
84+
columnInfo(mode, "USER()", "keyword", JDBCType.VARCHAR, 32766)));
8585
expected.put("rows", Arrays.asList(Arrays.asList(randomUserName)));
8686
Map<String, Object> actual = runSql(randomUserName, mode, SQL);
8787

@@ -97,7 +97,7 @@ public void testSingleRandomUserWithWhereEvaluatingTrue() throws IOException {
9797

9898
Map<String, Object> expected = new HashMap<>();
9999
expected.put("columns", Arrays.asList(
100-
columnInfo(mode, "USER()", "keyword", JDBCType.VARCHAR, 0)));
100+
columnInfo(mode, "USER()", "keyword", JDBCType.VARCHAR, 32766)));
101101
expected.put("rows", Arrays.asList(Arrays.asList(randomUserName),
102102
Arrays.asList(randomUserName),
103103
Arrays.asList(randomUserName)));
@@ -114,7 +114,7 @@ public void testSingleRandomUserWithWhereEvaluatingFalse() throws IOException {
114114

115115
Map<String, Object> expected = new HashMap<>();
116116
expected.put("columns", Arrays.asList(
117-
columnInfo(mode, "USER()", "keyword", JDBCType.VARCHAR, 0)));
117+
columnInfo(mode, "USER()", "keyword", JDBCType.VARCHAR, 32766)));
118118
expected.put("rows", Collections.<ArrayList<String>>emptyList());
119119
String anotherRandomUserName = randomValueOtherThan(randomUserName, () -> randomAlphaOfLengthBetween(1, 15));
120120
Map<String, Object> actual = runSql(randomUserName, mode, SQL + " FROM test WHERE USER()='" + anotherRandomUserName + "' LIMIT 3");
@@ -129,7 +129,7 @@ public void testMultipleRandomUsersAccess() throws IOException {
129129
Map<String, Object> expected = new HashMap<>();
130130

131131
expected.put("columns", Arrays.asList(
132-
columnInfo(mode, "USER()", "keyword", JDBCType.VARCHAR, 0)));
132+
columnInfo(mode, "USER()", "keyword", JDBCType.VARCHAR, 32766)));
133133
expected.put("rows", Arrays.asList(Arrays.asList(randomlyPickedUsername)));
134134
Map<String, Object> actual = runSql(randomlyPickedUsername, mode, SQL);
135135

@@ -147,7 +147,7 @@ public void testSingleUserSelectFromIndex() throws IOException {
147147

148148
Map<String, Object> expected = new HashMap<>();
149149
expected.put("columns", Arrays.asList(
150-
columnInfo(mode, "USER()", "keyword", JDBCType.VARCHAR, 0)));
150+
columnInfo(mode, "USER()", "keyword", JDBCType.VARCHAR, 32766)));
151151
expected.put("rows", Arrays.asList(Arrays.asList(randomUserName),
152152
Arrays.asList(randomUserName),
153153
Arrays.asList(randomUserName)));

‎x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/SqlProtocolTestCase.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@
2828
import java.util.Locale;
2929
import java.util.Map;
3030

31+
import static org.elasticsearch.xpack.sql.proto.Mode.CLI;
3132
import static org.elasticsearch.xpack.sql.proto.Protocol.SQL_QUERY_REST_ENDPOINT;
3233
import static org.elasticsearch.xpack.sql.proto.RequestInfo.CLIENT_IDS;
3334
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
34-
import static org.elasticsearch.xpack.sql.proto.Mode.CLI;
3535

3636
public abstract class SqlProtocolTestCase extends ESRestTestCase {
3737

@@ -62,7 +62,7 @@ public void testNumericTypes() throws IOException {
6262
}
6363

6464
public void testTextualType() throws IOException {
65-
assertQuery("SELECT 'abc123'", "'abc123'", "keyword", "abc123", 0);
65+
assertQuery("SELECT 'abc123'", "'abc123'", "keyword", "abc123", 32766);
6666
}
6767

6868
public void testDateTimes() throws IOException {
@@ -141,7 +141,7 @@ private void assertQuery(String sql, String columnName, String columnType, Objec
141141
List<Object> row = (ArrayList<Object>) rows.get(0);
142142
assertEquals(1, row.size());
143143

144-
// from xcontent we can get float or double, depending on the conversion
144+
// from xcontent we can get float or double, depending on the conversion
145145
// method of the specific xcontent format implementation
146146
if (columnValue instanceof Float && row.get(0) instanceof Double) {
147147
assertEquals(columnValue, (float)((Number) row.get(0)).doubleValue());
@@ -209,7 +209,7 @@ private Map<String, Object> runSql(String mode, String sql, boolean columnar) th
209209
return XContentHelper.convertToMap(SmileXContent.smileXContent, content, false);
210210
}
211211
default:
212-
return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false);
212+
return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false);
213213
}
214214
}
215215
}

‎x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public void testBasicQuery() throws IOException {
7474
String mode = randomMode();
7575
boolean columnar = randomBoolean();
7676

77-
expected.put("columns", singletonList(columnInfo(mode, "test", "text", JDBCType.VARCHAR, 0)));
77+
expected.put("columns", singletonList(columnInfo(mode, "test", "text", JDBCType.VARCHAR, Integer.MAX_VALUE)));
7878
if (columnar) {
7979
expected.put("values", singletonList(Arrays.asList("test", "test")));
8080
} else {
@@ -118,7 +118,7 @@ public void testNextPage() throws IOException {
118118
Map<String, Object> expected = new HashMap<>();
119119
if (i == 0) {
120120
expected.put("columns", Arrays.asList(
121-
columnInfo(mode, "text", "text", JDBCType.VARCHAR, 0),
121+
columnInfo(mode, "text", "text", JDBCType.VARCHAR, Integer.MAX_VALUE),
122122
columnInfo(mode, "number", "long", JDBCType.BIGINT, 20),
123123
columnInfo(mode, "s", "double", JDBCType.DOUBLE, 25),
124124
columnInfo(mode, "SCORE()", "float", JDBCType.REAL, 15)));
@@ -184,7 +184,7 @@ public void testScoreWithFieldNamedScore() throws IOException {
184184
Map<String, Object> expected = new HashMap<>();
185185
boolean columnar = randomBoolean();
186186
expected.put("columns", Arrays.asList(
187-
columnInfo(mode, "name", "text", JDBCType.VARCHAR, 0),
187+
columnInfo(mode, "name", "text", JDBCType.VARCHAR, Integer.MAX_VALUE),
188188
columnInfo(mode, "score", "long", JDBCType.BIGINT, 20),
189189
columnInfo(mode, "SCORE()", "float", JDBCType.REAL, 15)));
190190
if (columnar) {
@@ -427,7 +427,7 @@ public void testBasicQueryWithFilter() throws IOException {
427427
"{\"test\":\"bar\"}");
428428

429429
Map<String, Object> expected = new HashMap<>();
430-
expected.put("columns", singletonList(columnInfo(mode, "test", "text", JDBCType.VARCHAR, 0)));
430+
expected.put("columns", singletonList(columnInfo(mode, "test", "text", JDBCType.VARCHAR, Integer.MAX_VALUE)));
431431
expected.put("rows", singletonList(singletonList("foo")));
432432
assertResponse(expected, runSql(new StringEntity("{\"query\":\"SELECT * FROM test\", " +
433433
"\"filter\":{\"match\": {\"test\": \"foo\"}}" + mode(mode) + "}",
@@ -442,7 +442,7 @@ public void testBasicQueryWithParameters() throws IOException {
442442

443443
Map<String, Object> expected = new HashMap<>();
444444
expected.put("columns", Arrays.asList(
445-
columnInfo(mode, "test", "text", JDBCType.VARCHAR, 0),
445+
columnInfo(mode, "test", "text", JDBCType.VARCHAR, Integer.MAX_VALUE),
446446
columnInfo(mode, "param", "integer", JDBCType.INTEGER, 11)
447447
));
448448
if (columnar) {

‎x-pack/plugin/sql/qa/src/main/resources/setup_mock_metadata_get_columns.sql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ CREATE TABLE mock (
2323
IS_AUTOINCREMENT VARCHAR,
2424
IS_GENERATEDCOLUMN VARCHAR
2525
) AS
26-
SELECT null, 'test1', 'name', 12, 'TEXT', 0, 2147483647, null, null,
26+
SELECT null, 'test1', 'name', 12, 'TEXT', 2147483647, 2147483647, null, null,
2727
1, -- columnNullable
2828
null, null, 12, 0, 2147483647, 1, 'YES', null, null, null, null, 'NO', 'NO'
2929
FROM DUAL
3030
UNION ALL
31-
SELECT null, 'test1', 'name.keyword', 12, 'KEYWORD', 0, 2147483647, null, null,
31+
SELECT null, 'test1', 'name.keyword', 12, 'KEYWORD', 32766, 2147483647, null, null,
3232
1, -- columnNullable
3333
null, null, 12, 0, 2147483647, 1, 'YES', null, null, null, null, 'NO', 'NO'
3434
FROM DUAL

‎x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/proto/ColumnInfo.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public boolean equals(Object o) {
119119
return false;
120120
}
121121
ColumnInfo that = (ColumnInfo) o;
122-
return displaySize == that.displaySize &&
122+
return Objects.equals(displaySize, that.displaySize) &&
123123
Objects.equals(table, that.table) &&
124124
Objects.equals(name, that.name) &&
125125
Objects.equals(esType, that.esType);

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,7 @@ public final void execute(SqlSession session, ActionListener<SchemaRowSet> liste
8282
.sorted(Comparator.comparing((DataType t) -> t.sqlType.getVendorTypeNumber()).thenComparing(DataType::sqlName))
8383
.map(t -> asList(t.toString(),
8484
t.sqlType.getVendorTypeNumber(),
85-
//https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/column-size?view=sql-server-2017
86-
t.defaultPrecision,
85+
DataTypes.precision(t),
8786
"'",
8887
"'",
8988
null,

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,19 @@ public enum DataType {
3636
DOUBLE( "double", JDBCType.DOUBLE, Double.BYTES, 15, 25, false, true, true),
3737
// 24 bits defaultPrecision - 24*log10(2) =~ 7 (7.22)
3838
FLOAT( "float", JDBCType.REAL, Float.BYTES, 7, 15, false, true, true),
39-
HALF_FLOAT( "half_float", JDBCType.FLOAT, Double.BYTES, 16, 25, false, true, true),
39+
HALF_FLOAT( "half_float", JDBCType.FLOAT, Float.BYTES, 3, 25, false, true, true),
4040
// precision is based on long
41-
SCALED_FLOAT( "scaled_float", JDBCType.FLOAT, Double.BYTES, 19, 25, false, true, true),
42-
KEYWORD( "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE, 256, 0, false, false, true),
43-
TEXT( "text", JDBCType.VARCHAR, Integer.MAX_VALUE, Integer.MAX_VALUE, 0, false, false, false),
41+
SCALED_FLOAT( "scaled_float", JDBCType.DOUBLE, Long.BYTES, 15, 25, false, true, true),
42+
KEYWORD( "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE, 32766, 32766, false, false, true),
43+
TEXT( "text", JDBCType.VARCHAR, Integer.MAX_VALUE, Integer.MAX_VALUE, Integer.MAX_VALUE, false, false, false),
4444
OBJECT( "object", JDBCType.STRUCT, -1, 0, 0, false, false, false),
4545
NESTED( "nested", JDBCType.STRUCT, -1, 0, 0, false, false, false),
46-
BINARY( "binary", JDBCType.VARBINARY, -1, Integer.MAX_VALUE, 0, false, false, false),
46+
BINARY( "binary", JDBCType.VARBINARY, -1, Integer.MAX_VALUE, Integer.MAX_VALUE, false, false, false),
4747
DATE( JDBCType.DATE, Long.BYTES, 24, 24, false, false, true),
4848
// since ODBC and JDBC interpret precision for Date as display size
4949
// the precision is 23 (number of chars in ISO8601 with millis) + Z (the UTC timezone)
5050
// see https://github.com/elastic/elasticsearch/issues/30386#issuecomment-386807288
51-
DATETIME( "date", JDBCType.TIMESTAMP, Long.BYTES, 24, 24, false, false, true),
51+
DATETIME( "date", JDBCType.TIMESTAMP, Long.BYTES, 3, 24, false, false, true),
5252
//
5353
// specialized types
5454
//

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

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public static Integer metaSqlDataType(DataType t) {
175175
}
176176

177177
// https://github.com/elastic/elasticsearch/issues/30386
178-
// https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function?view=sql-server-2017
178+
// https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function
179179
public static Integer metaSqlDateTimeSub(DataType t) {
180180
if (t == DATETIME) {
181181
// ODBC SQL_CODE_TIMESTAMP
@@ -185,42 +185,44 @@ public static Integer metaSqlDateTimeSub(DataType t) {
185185
return 0;
186186
}
187187

188-
// https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/decimal-digits?view=sql-server-2017
189188
public static Short metaSqlMinimumScale(DataType t) {
190-
// TODO: return info for HALF/SCALED_FLOATS (should be based on field not type)
191-
if (t == DATETIME) {
192-
return Short.valueOf((short) 3);
193-
}
194-
if (t.isInteger()) {
195-
return Short.valueOf((short) 0);
196-
}
197-
// minimum scale?
198-
if (t.isRational()) {
199-
return Short.valueOf((short) 0);
200-
}
201-
return null;
189+
return metaSqlSameScale(t);
202190
}
203191

204192
public static Short metaSqlMaximumScale(DataType t) {
205-
// TODO: return info for HALF/SCALED_FLOATS (should be based on field not type)
206-
if (t == DATETIME) {
207-
return Short.valueOf((short) 3);
208-
}
193+
return metaSqlSameScale(t);
194+
}
195+
196+
// https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/decimal-digits
197+
// https://github.com/elastic/elasticsearch/issues/40357
198+
// since the scale is fixed, minimum and maximum should return the same value
199+
// hence why this method exists
200+
private static Short metaSqlSameScale(DataType t) {
201+
// TODO: return info for SCALED_FLOATS (should be based on field not type)
209202
if (t.isInteger()) {
210203
return Short.valueOf((short) 0);
211204
}
212-
if (t.isRational()) {
205+
if (t.isDateBased() || t.isRational()) {
213206
return Short.valueOf((short) t.defaultPrecision);
214207
}
215208
return null;
216209
}
217210

218-
// https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function?view=sql-server-2017
211+
// https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function
219212
public static Integer metaSqlRadix(DataType t) {
220213
// RADIX - Determines how numbers returned by COLUMN_SIZE and DECIMAL_DIGITS should be interpreted.
221214
// 10 means they represent the number of decimal digits allowed for the column.
222215
// 2 means they represent the number of bits allowed for the column.
223216
// null means radix is not applicable for the given type.
224217
return t.isInteger() ? Integer.valueOf(10) : (t.isRational() ? Integer.valueOf(2) : null);
225218
}
219+
220+
//https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function#comments
221+
//https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/column-size
222+
public static Integer precision(DataType t) {
223+
if (t.isNumeric()) {
224+
return t.defaultPrecision;
225+
}
226+
return t.displaySize;
227+
}
226228
}

0 commit comments

Comments
 (0)