-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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: Fix esType for DATETIME/DATE and INTERVALS #38179
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5c7ff99
SQL: Fix esType for DATETIME/DATE and INTERVALS
matriv 508f53f
Address comment
matriv 7dd0a81
Merge remote-tracking branch 'upstream/master' into mt/fix-38051
matriv ea18004
Fix: use correct field
matriv 3057fd7
Address comment
matriv 4fed2fb
fixup! Address comment
matriv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,41 +23,41 @@ | |
public enum DataType { | ||
|
||
// @formatter:off | ||
// jdbc type, size, defPrecision,dispSize, int, rat, docvals | ||
NULL( JDBCType.NULL, 0, 0, 0, false, false, false), | ||
UNSUPPORTED( JDBCType.OTHER, 0, 0, 0, false, false, false), | ||
BOOLEAN( JDBCType.BOOLEAN, 1, 1, 1, false, false, false), | ||
BYTE( JDBCType.TINYINT, Byte.BYTES, 3, 5, true, false, true), | ||
SHORT( JDBCType.SMALLINT, Short.BYTES, 5, 6, true, false, true), | ||
INTEGER( JDBCType.INTEGER, Integer.BYTES, 10, 11, true, false, true), | ||
LONG( JDBCType.BIGINT, Long.BYTES, 19, 20, true, false, true), | ||
// esType jdbc type, size, defPrecision,dispSize, int, rat, docvals | ||
NULL( "null", JDBCType.NULL, 0, 0, 0, false, false, false), | ||
UNSUPPORTED( JDBCType.OTHER, 0, 0, 0, false, false, false), | ||
BOOLEAN( "boolean", JDBCType.BOOLEAN, 1, 1, 1, false, false, false), | ||
BYTE( "byte", JDBCType.TINYINT, Byte.BYTES, 3, 5, true, false, true), | ||
SHORT( "short", JDBCType.SMALLINT, Short.BYTES, 5, 6, true, false, true), | ||
INTEGER( "integer", JDBCType.INTEGER, Integer.BYTES, 10, 11, true, false, true), | ||
LONG( "long", JDBCType.BIGINT, Long.BYTES, 19, 20, true, false, true), | ||
// 53 bits defaultPrecision ~ 15(15.95) decimal digits (53log10(2)), | ||
DOUBLE( JDBCType.DOUBLE, Double.BYTES, 15, 25, false, true, true), | ||
DOUBLE( "double", JDBCType.DOUBLE, Double.BYTES, 15, 25, false, true, true), | ||
// 24 bits defaultPrecision - 24*log10(2) =~ 7 (7.22) | ||
FLOAT( JDBCType.REAL, Float.BYTES, 7, 15, false, true, true), | ||
HALF_FLOAT( JDBCType.FLOAT, Double.BYTES, 16, 25, false, true, true), | ||
FLOAT( "float", JDBCType.REAL, Float.BYTES, 7, 15, false, true, true), | ||
HALF_FLOAT( "half_float", JDBCType.FLOAT, Double.BYTES, 16, 25, false, true, true), | ||
// precision is based on long | ||
SCALED_FLOAT( JDBCType.FLOAT, Double.BYTES, 19, 25, false, true, true), | ||
KEYWORD( JDBCType.VARCHAR, Integer.MAX_VALUE, 256, 0, false, false, true), | ||
TEXT( JDBCType.VARCHAR, Integer.MAX_VALUE, Integer.MAX_VALUE, 0, false, false, false), | ||
OBJECT( JDBCType.STRUCT, -1, 0, 0, false, false, false), | ||
NESTED( JDBCType.STRUCT, -1, 0, 0, false, false, false), | ||
BINARY( JDBCType.VARBINARY, -1, Integer.MAX_VALUE, 0, false, false, false), | ||
DATE( JDBCType.DATE, Long.BYTES, 10, 10, false, false, true), | ||
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), | ||
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), | ||
DATE( JDBCType.DATE, Long.BYTES, 10, 10, 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. Shouldn't this be "date" as well? 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. But |
||
// 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( JDBCType.TIMESTAMP, Long.BYTES, 24, 24, false, false, true), | ||
DATETIME( "date", JDBCType.TIMESTAMP, Long.BYTES, 24, 24, false, false, true), | ||
// | ||
// specialized types | ||
// | ||
// 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 | ||
// aka 39 chars | ||
IP( JDBCType.VARCHAR, 39, 39, 0, false, false, true), | ||
IP( "ip", JDBCType.VARCHAR, 39, 39, 0, false, false, true), | ||
// | ||
// INTERVALS | ||
// the list is long as there are a lot of variations and that's what clients (ODBC) expect | ||
// jdbc type, size, prec,disp, int, rat, docvals | ||
// esType:null jdbc type, size, prec,disp, int, rat, docvals | ||
INTERVAL_YEAR( ExtTypes.INTERVAL_YEAR, Integer.BYTES, 7, 7, false, false, false), | ||
INTERVAL_MONTH( ExtTypes.INTERVAL_MONTH, Integer.BYTES, 7, 7, false, false, false), | ||
INTERVAL_DAY( ExtTypes.INTERVAL_DAY, Long.BYTES, 23, 23, false, false, false), | ||
|
@@ -126,7 +126,12 @@ public enum DataType { | |
|
||
|
||
/** | ||
* Elasticsearch type name | ||
* Type's name used for error messages and column info for the clients | ||
*/ | ||
public final String typeName; | ||
|
||
/** | ||
* Elasticsearch data type that it maps to | ||
*/ | ||
public final String esType; | ||
|
||
|
@@ -176,7 +181,13 @@ public enum DataType { | |
|
||
DataType(SQLType sqlType, int size, int defaultPrecision, int displaySize, boolean isInteger, | ||
boolean isRational, boolean defaultDocValues) { | ||
this.esType = name().toLowerCase(Locale.ROOT); | ||
this(null, sqlType, size, defaultPrecision, displaySize, isInteger, isRational, defaultDocValues); | ||
} | ||
|
||
DataType(String esType, SQLType sqlType, int size, int defaultPrecision, int displaySize, boolean isInteger, | ||
boolean isRational, boolean defaultDocValues) { | ||
this.typeName = name().toLowerCase(Locale.ROOT); | ||
this.esType = esType; | ||
this.sqlType = sqlType; | ||
this.size = size; | ||
this.defaultPrecision = defaultPrecision; | ||
|
@@ -228,8 +239,6 @@ public static DataType fromOdbcType(String odbcType) { | |
|
||
/** | ||
* Creates returns DataType enum corresponding to the specified es type | ||
* <p> | ||
* For any dataType DataType.fromTypeName(dataType.esType) == dataType | ||
*/ | ||
public static DataType fromTypeName(String esType) { | ||
String uppercase = esType.toUpperCase(Locale.ROOT); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I would have gone the other way - use the lower-case enum way as a default type and just override this convention with null or "date". Just a matter of preference really.
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.
it's more elegant yes, but I choose the more "explicit" way to show clearly what the corresponding esType is.