Skip to content

SQL: Implement CONVERT, an alternative to CAST#34660

Merged
matriv merged 9 commits intoelastic:masterfrom
matriv:mt/impl-34513
Oct 23, 2018
Merged

SQL: Implement CONVERT, an alternative to CAST#34660
matriv merged 9 commits intoelastic:masterfrom
matriv:mt/impl-34513

Conversation

@matriv
Copy link
Copy Markdown
Contributor

@matriv matriv commented Oct 19, 2018

CONVERT works exactly like cast with slightly different syntax:
CONVERT(<value>, <data_type) as opposed to CAST(<value> AS <data_type>)

Moreover it support format of the MS-SQL data types SQL_<type>,
e.g.: SQL_INTEGER

Closes: #34513

@matriv
SQL: Implement CONVERT, an alternative to CAST
f93811d 
`CONVERT` works exactly like cast with slightly different syntax:
`CONVERT(<value>, <data_type)` as opposed to `CAST(<value> AS <data_type>)`

Moreover it support format of the MS-SQL data types `SQL_<type>`,
e.g.: `SQL_INTEGER`

Closes: elastic#34513
@matriv matriv requested review from astefan and costin October 19, 2018 22:21
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Oct 19, 2018

Pinging @elastic/es-search-aggs

@matriv
Copy link
Copy Markdown
Contributor Author

matriv commented Oct 19, 2018

@costin @astefan Instead of null here: https://github.com/elastic/elasticsearch/pull/34660/files#diff-eff884b6c6b53d7e75ebac8b6a270830R92 we can also put UNSUPPORTED.

In this case, the user won't get a ParsingException but a TypeResolution coming from Cast when tries to do the conversion. Do you think this is preferable?

@astefan
Copy link
Copy Markdown
Contributor

astefan commented Oct 21, 2018

Definitely UNSUPPORTED from usability point of view. It is not an invalid data type, but rather a valid one we do not support yet.

costin
costin approved these changes Oct 21, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@costin costin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
+1 on using UNSUPPORTED instead of null.

x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/type/DataType.java Outdated
DATE( JDBCType.TIMESTAMP, Timestamp.class, Long.BYTES, 24, 24);
// @formatter:on

public static final String MSSQL_DATATYPE_PREFIX = "SQL_";
Copy link
Copy Markdown
Member

@costin costin Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not so much MSSQL as ODBC spec.

Copy link
Copy Markdown
Contributor Author

@matriv matriv Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, replaced all MSSQL with ODBC.

astefan
astefan approved these changes Oct 21, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@astefan astefan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left just one minor comment related to docs.

x-pack/qa/sql/src/main/resources/docs.csv-spec Outdated
;


conversionStringToIntConvertESDataType
Copy link
Copy Markdown
Contributor

@astefan astefan Oct 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put this example first and the odbc one second, in order of familiarity/usage?

@matriv
Copy link
Copy Markdown
Contributor Author

matriv commented Oct 22, 2018

retest this please

1 similar comment
@matriv
Copy link
Copy Markdown
Contributor Author

matriv commented Oct 22, 2018

retest this please

@costin
Copy link
Copy Markdown
Member

costin commented Oct 22, 2018

Try merging master again - hopefully it fixes the external failure.

@matriv matriv merged commit e9e1407 into elastic:master Oct 23, 2018
matriv pushed a commit that referenced this pull request Oct 23, 2018
@matriv
SQL: Implement CONVERT, an alternative to CAST (#34660)
3a3c95c 
`CONVERT` works exactly like cast with slightly different syntax:
`CONVERT(<value>, <data_type)` as opposed to `CAST(<value> AS <data_type>)`

Moreover it support format of the MS-SQL data types `SQL_<type>`,
e.g.: `SQL_INTEGER`

Closes: #34513
@matriv matriv deleted the mt/impl-34513 branch October 23, 2018 09:27
@matriv
Copy link
Copy Markdown
Contributor Author

matriv commented Oct 23, 2018

Backported to 6.x with 3a3c95c

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Oct 23, 2018
@jasontedor
Merge branch 'master' into ccr-docs
f7bbdd1 
* master: (24 commits)
  ingest: better support for conditionals with simulate?verbose (elastic#34155)
  [Rollup] Job deletion should be invoked on the allocated task (elastic#34574)
  [DOCS] .Security index is never auto created (elastic#34589)
  CCR: Requires soft-deletes on the follower (elastic#34725)
  re-enable bwc tests (elastic#34743)
  Empty GetAliases authorization fix (elastic#34444)
  INGEST: Document Processor Conditional (elastic#33388)
  [CCR] Add total fetch time leader stat (elastic#34577)
  SQL: Support pattern against compatible indices (elastic#34718)
  [CCR] Auto follow pattern APIs adjustments (elastic#34518)
  [Test] Remove dead code from ExceptionSerializationTests (elastic#34713)
  A small typo in migration-assistance doc (elastic#34704)
  ingest: processor stats (elastic#34724)
  SQL: Implement IN(value1, value2, ...) expression. (elastic#34581)
  Tests: Add checks to GeoDistanceQueryBuilderTests (elastic#34273)
  INGEST: Rename Pipeline Processor Param. (elastic#34733)
  Core: Move IndexNameExpressionResolver to java time (elastic#34507)
  [DOCS] Force Merge: clarify execution and storage requirements (elastic#33882)
  TESTING.asciidoc fix examples using forbidden annotation (elastic#34515)
  SQL: Implement `CONVERT`, an alternative to `CAST` (elastic#34660)
  ...
kcm pushed a commit that referenced this pull request Oct 30, 2018
@kcm
SQL: Implement CONVERT, an alternative to CAST (#34660)
9ebbdf0 
`CONVERT` works exactly like cast with slightly different syntax:
`CONVERT(<value>, <data_type)` as opposed to `CAST(<value> AS <data_type>)`

Moreover it support format of the MS-SQL data types `SQL_<type>`,
e.g.: `SQL_INTEGER`

Closes: #34513
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@costin costin costin approved these changes

@astefan astefan astefan approved these changes

Assignees

No one assigned

Labels

:Analytics/SQL SQL querying >enhancement >feature v6.5.0 v7.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@matriv @elasticmachine @astefan @costin @colings86