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

Fix issue with GROUP BY YEAR() #49559

Merged
merged 1 commit into from
Nov 26, 2019
Merged

Fix issue with GROUP BY YEAR() #49559

merged 1 commit into from
Nov 26, 2019

Conversation

matriv
Copy link
Contributor

@matriv matriv commented Nov 25, 2019

Grouping By YEAR() is translated to a histogram aggregation, but
previously if there was a scalar function invloved (e.g.:
YEAR(date + INTERVAL 2 YEARS)), there was no proper script created
and the histogram was applied on a field with name: date + INTERVAL 2 YEARS
which doesn't make sense, and resulted in null result.

Check the underlying field of YEAR() and if it's a function call
asScript() to properly get the painless script on which the histogram
is applied.

Fixes: #49386

Grouping By YEAR() is translated to a histogram aggregation, but
previously if there was a scalar function invloved (e.g.:
`YEAR(date + INTERVAL 2 YEARS)`), there was no proper script created
and the histogram was applied on a field with name: `date + INTERVAL 2 YEARS`
which doesn't make sense, and resulted in null result.

Check the underlying field of YEAR() and if it's a function call
`asScript()` to properly get the painless script on which the histogram
is applied.

Fixes: elastic#49386
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/SQL)

@matriv
Copy link
Contributor Author

matriv commented Nov 25, 2019

I have to test it but I think this was also problematic before the change to use calendar histogram for YEAR(). Do you think it worths fixing it for 7.4 and 6.8 too @costin @astefan ?

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

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

@astefan
Copy link
Contributor

astefan commented Nov 26, 2019

I would put it on 6.8, as well.

@matriv matriv merged commit 93c37ab into elastic:master Nov 26, 2019
@matriv matriv deleted the fix-49386 branch November 26, 2019 12:23
matriv added a commit that referenced this pull request Nov 26, 2019
Grouping By YEAR() is translated to a histogram aggregation, but
previously if there was a scalar function invloved (e.g.:
`YEAR(date + INTERVAL 2 YEARS)`), there was no proper script created
and the histogram was applied on a field with name: `date + INTERVAL 2 YEARS`
which doesn't make sense, and resulted in null result.

Check the underlying field of YEAR() and if it's a function call
`asScript()` to properly get the painless script on which the histogram
is applied.

Fixes: #49386
(cherry picked from commit 93c37ab)
matriv added a commit that referenced this pull request Nov 26, 2019
Grouping By YEAR() is translated to a histogram aggregation, but
previously if there was a scalar function invloved (e.g.:
`YEAR(date + INTERVAL 2 YEARS)`), there was no proper script created
and the histogram was applied on a field with name: `date + INTERVAL 2 YEARS`
which doesn't make sense, and resulted in null result.

Check the underlying field of YEAR() and if it's a function call
`asScript()` to properly get the painless script on which the histogram
is applied.

Fixes: #49386
(cherry picked from commit 93c37ab)
matriv added a commit that referenced this pull request Nov 26, 2019
Grouping By YEAR() is translated to a histogram aggregation, but
previously if there was a scalar function invloved (e.g.:
`YEAR(date + INTERVAL 2 YEARS)`), there was no proper script created
and the histogram was applied on a field with name: `date + INTERVAL 2 YEARS`
which doesn't make sense, and resulted in null result.

Check the underlying field of YEAR() and if it's a function call
`asScript()` to properly get the painless script on which the histogram
is applied.

Fixes: #49386
(cherry picked from commit 93c37ab)
matriv added a commit that referenced this pull request Nov 26, 2019
Grouping By YEAR() is translated to a histogram aggregation, but
previously if there was a scalar function invloved (e.g.:
`YEAR(date + INTERVAL 2 YEARS)`), there was no proper script created
and the histogram was applied on a field with name: `date + INTERVAL 2 YEARS`
which doesn't make sense, and resulted in null result.

Check the underlying field of YEAR() and if it's a function call
`asScript()` to properly get the painless script on which the histogram
is applied.

Fixes: #49386
(cherry picked from commit 93c37ab)
(cherry picked from commit e54d7d5)
@jpountz jpountz changed the title SQL: Fix issue with GROUP BY YEAR() Fix issue with GROUP BY YEAR() Dec 18, 2019
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: GROUPing BY YEAR function applied on a CASTed AS DATE field results in NULL values
5 participants