Skip to content
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 issue with common type resolution #46565

Merged
merged 1 commit into from Sep 11, 2019
Merged

Conversation

matriv
Copy link
Contributor

@matriv matriv commented Sep 10, 2019

Many scalar functions try to find out the common type between their
arguments in order to set it as their return time, e.g.:
for + the common type which is set as the return type
of the + operation is double.

Previously, for data types TEXT and KEYWORD (string data types) there
was no common data type found and null was returned causing NPEs when
the function was trying to resolve the return data type.

Fixes: #46551

Many scalar functions try to find out the common type between their
arguments in order to set it as their return time, e.g.:
for <float> + <double> the common type which is set as the return type
of the + operation is double.

Previously, for data types TEXT and KEYWORD (string data types) there
was no common data type found and null was returned causing NPEs when
the function was trying to resolve the return data type.

Fixes: elastic#46551
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@matriv
Copy link
Contributor Author

matriv commented Sep 10, 2019

@elasticmachine run elasticsearch-ci/2

@@ -50,6 +51,12 @@ public static DataType commonType(DataType left, DataType right) {
if (DataTypes.isNull(right)) {
return left;
}
if (left.isString() && right.isString()) {
if (left == TEXT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why TEXT and not KEYWORD?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought that TEXT is "larger" as double > float, and TEXT is better to be exposed in case the function is used somewhere that we need to check for exact/inexact.

Copy link
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 but left one comment.

Copy link
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

@matriv matriv merged commit 291017d into elastic:master Sep 11, 2019
@matriv matriv deleted the fix-46551 branch September 11, 2019 15:27
matriv added a commit that referenced this pull request Sep 11, 2019
Many scalar functions try to find out the common type between their
arguments in order to set it as their return time, e.g.:
for `float + double` the common type which is set as the return type
of the + operation is `double`.

Previously, for data types TEXT and KEYWORD (string data types) there
was no common data type found and null was returned causing NPEs when
the function was trying to resolve the return data type.

Fixes: #46551
(cherry picked from commit 291017d)
matriv added a commit that referenced this pull request Sep 11, 2019
Many scalar functions try to find out the common type between their
arguments in order to set it as their return time, e.g.:
for `float + double` the common type which is set as the return type
of the + operation is `double`.

Previously, for data types TEXT and KEYWORD (string data types) there
was no common data type found and null was returned causing NPEs when
the function was trying to resolve the return data type.

Fixes: #46551
(cherry picked from commit 291017d)
matriv added a commit that referenced this pull request Sep 11, 2019
Many scalar functions try to find out the common type between their
arguments in order to set it as their return time, e.g.:
for `float + double` the common type which is set as the return type
of the + operation is `double`.

Previously, for data types TEXT and KEYWORD (string data types) there
was no common data type found and null was returned causing NPEs when
the function was trying to resolve the return data type.

Fixes: #46551
(cherry picked from commit 291017d)
matriv added a commit that referenced this pull request Sep 11, 2019
Many scalar functions try to find out the common type between their
arguments in order to set it as their return time, e.g.:
for `float + double` the common type which is set as the return type
of the + operation is `double`.

Previously, for data types TEXT and KEYWORD (string data types) there
was no common data type found and null was returned causing NPEs when
the function was trying to resolve the return data type.

Fixes: #46551
(cherry picked from commit 291017d)
@matriv matriv added the v6.8.4 label Sep 11, 2019
turackangal pushed a commit to turackangal/elasticsearch that referenced this pull request Sep 14, 2019
Many scalar functions try to find out the common type between their
arguments in order to set it as their return time, e.g.:
for `float + double` the common type which is set as the return type
of the + operation is `double`.

Previously, for data types TEXT and KEYWORD (string data types) there
was no common data type found and null was returned causing NPEs when
the function was trying to resolve the return data type.

Fixes: elastic#46551
@colings86 colings86 added v7.4.0 and removed v7.4.1 labels Sep 17, 2019
jkakavas pushed a commit to jkakavas/elasticsearch that referenced this pull request Sep 25, 2019
Many scalar functions try to find out the common type between their
arguments in order to set it as their return time, e.g.:
for `float + double` the common type which is set as the return type
of the + operation is `double`.

Previously, for data types TEXT and KEYWORD (string data types) there
was no common data type found and null was returned causing NPEs when
the function was trying to resolve the return data type.

Fixes: elastic#46551
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SQL: NPE when using CASE WHEN with CASTed returned value
6 participants