-
Notifications
You must be signed in to change notification settings - Fork 24.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SQL: Adjust the precision and scale for drivers #40467
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,19 +36,19 @@ public enum DataType { | |
DOUBLE( "double", JDBCType.DOUBLE, Double.BYTES, 15, 25, false, true, true), | ||
// 24 bits defaultPrecision - 24*log10(2) =~ 7 (7.22) | ||
FLOAT( "float", JDBCType.REAL, Float.BYTES, 7, 15, false, true, true), | ||
HALF_FLOAT( "half_float", JDBCType.FLOAT, Double.BYTES, 16, 25, false, true, true), | ||
HALF_FLOAT( "half_float", JDBCType.FLOAT, Float.BYTES, 3, 25, false, true, true), | ||
// precision is based on long | ||
SCALED_FLOAT( "scaled_float", JDBCType.FLOAT, Double.BYTES, 19, 25, false, true, true), | ||
KEYWORD( "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE, 256, 0, false, false, true), | ||
TEXT( "text", JDBCType.VARCHAR, Integer.MAX_VALUE, Integer.MAX_VALUE, 0, false, false, false), | ||
SCALED_FLOAT( "scaled_float", JDBCType.DOUBLE, Integer.BYTES, 15, 25, false, true, true), | ||
KEYWORD( "keyword", JDBCType.VARCHAR, Integer.MAX_VALUE, 256, 32766,false, false, true), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing whitespace after |
||
TEXT( "text", JDBCType.VARCHAR, Integer.MAX_VALUE, Integer.MAX_VALUE, Integer.MAX_VALUE, false, false, false), | ||
OBJECT( "object", JDBCType.STRUCT, -1, 0, 0, false, false, false), | ||
NESTED( "nested", JDBCType.STRUCT, -1, 0, 0, false, false, false), | ||
BINARY( "binary", JDBCType.VARBINARY, -1, Integer.MAX_VALUE, 0, false, false, false), | ||
BINARY( "binary", JDBCType.VARBINARY, -1, Integer.MAX_VALUE, Integer.MAX_VALUE, false, false, false), | ||
DATE( JDBCType.DATE, Long.BYTES, 24, 24, false, false, true), | ||
// 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 | ||
DATETIME( "date", JDBCType.TIMESTAMP, Long.BYTES, 24, 24, false, false, true), | ||
DATETIME( "date", JDBCType.TIMESTAMP, Long.BYTES, 3, 24, false, false, true), | ||
// | ||
// specialized types | ||
// | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -175,7 +175,7 @@ public static Integer metaSqlDataType(DataType t) { | |
} | ||
|
||
// https://github.com/elastic/elasticsearch/issues/30386 | ||
// https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function?view=sql-server-2017 | ||
// https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function | ||
public static Integer metaSqlDateTimeSub(DataType t) { | ||
if (t == DATETIME) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also here: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this was skipped but shouldn't be changed too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so since the type info indicates it should be just for datetime (timestamp) not DATE or TIME |
||
// ODBC SQL_CODE_TIMESTAMP | ||
|
@@ -185,42 +185,44 @@ public static Integer metaSqlDateTimeSub(DataType t) { | |
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 == DATETIME) { | ||
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; | ||
return metaSqlSameScale(t); | ||
} | ||
|
||
public static Short metaSqlMaximumScale(DataType t) { | ||
// TODO: return info for HALF/SCALED_FLOATS (should be based on field not type) | ||
if (t == DATETIME) { | ||
return Short.valueOf((short) 3); | ||
} | ||
return metaSqlSameScale(t); | ||
} | ||
|
||
// https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/decimal-digits | ||
// https://github.com/elastic/elasticsearch/issues/40357 | ||
// since the scale is fixed, minimum and maximum should return the same value | ||
// hence why this method exists | ||
private static Short metaSqlSameScale(DataType t) { | ||
// TODO: return info for SCALED_FLOATS (should be based on field not type) | ||
if (t.isInteger()) { | ||
return Short.valueOf((short) 0); | ||
} | ||
if (t.isRational()) { | ||
if (t == DATETIME || t.isRational()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be: |
||
return Short.valueOf((short) t.defaultPrecision); | ||
} | ||
return null; | ||
} | ||
|
||
// https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function?view=sql-server-2017 | ||
// https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function | ||
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); | ||
} | ||
|
||
//https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgettypeinfo-function#comments | ||
//https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/column-size | ||
public static Integer precision(DataType t) { | ||
if (t.isNumeric()) { | ||
return t.defaultPrecision; | ||
} | ||
return t.displaySize; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is
Integer.BYTES
correct here? because the comment above says:precision is based on long