Skip to content

SQL: Handle aggregation for null group#34916

Merged
costin merged 2 commits intoelastic:masterfrom
costin:fix-34896
Oct 26, 2018
Merged

SQL: Handle aggregation for null group#34916
costin merged 2 commits intoelastic:masterfrom
costin:fix-34896

Conversation

@costin
Copy link
Copy Markdown
Member

@costin costin commented Oct 26, 2018

When dealing with a null group, make sure the associated aggs return
null

Fix #34896

@costin
SQL: Handle aggregation for null group
41e0976 
When dealing with a null group, make sure the associated aggs return
null

Fix elastic#34896
@costin costin requested review from astefan and matriv October 26, 2018 15:13
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Oct 26, 2018

Pinging @elastic/es-search-aggs

astefan
astefan approved these changes Oct 26, 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

...src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/MetricAggExtractor.java Outdated
* Check if the given aggregate has been executed and has computed values
* or not (the bucket is null).
*
* Hopefully solved by
Copy link
Copy Markdown
Contributor

@astefan astefan Oct 26, 2018

Choose a reason for hiding this comment

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

Incomplete statement?

Copy link
Copy Markdown
Contributor

@matriv matriv Oct 26, 2018

Choose a reason for hiding this comment

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

Please complete the sentence ;-)

matriv
matriv approved these changes Oct 26, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@matriv matriv left a comment

Choose a reason for hiding this comment

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

LGTM

@costin costin added the v6.6.0 label Oct 26, 2018
@costin costin merged commit 55d48e4 into elastic:master Oct 26, 2018
@costin costin deleted the fix-34896 branch October 26, 2018 18:34
costin added a commit that referenced this pull request Oct 26, 2018
@costin
SQL: Handle aggregation for null group (#34916)
c4cc3d5 
When dealing with a null group, the associated metric aggs need to
return null as well

Fix #34896

(cherry picked from commit 55d48e4)
costin added a commit that referenced this pull request Oct 28, 2018
@costin
SQL: Handle aggregation for null group (#34916)
63d8569 
When dealing with a null group, the associated metric aggs need to
return null as well

Fix #34896

(cherry picked from commit 55d48e4)
kcm pushed a commit that referenced this pull request Oct 30, 2018
@costin @kcm
SQL: Handle aggregation for null group (#34916)
056dac3 
When dealing with a null group, the associated metric aggs need to
return null as well

Fix #34896
@colings86 colings86 added v6.5.0 and removed v6.5.1 labels Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@astefan astefan astefan approved these changes

+1 more reviewer

@matriv matriv matriv approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

:Analytics/SQL SQL querying >bug v6.5.0 v6.6.0 v7.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@costin @elasticmachine @astefan @matriv @colings86