Skip to content

SQL: Streamline declaration of LeafAggs#55380

Merged
costin merged 2 commits intoelastic:masterfrom
costin:sql/streamline-leafaggs
Apr 17, 2020
Merged

SQL: Streamline declaration of LeafAggs#55380
costin merged 2 commits intoelastic:masterfrom
costin:sql/streamline-leafaggs

Conversation

@costin
Copy link
Copy Markdown
Member

@costin costin commented Apr 17, 2020

Avoid repetition of the aggregation builder setup

Relates #55241

Avoid repetition of the aggregation builder setup

Relates elastic#55241
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-ql (:Query Languages/SQL)

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.

Very nice! Thx! LGTM

this.percents = percents;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure why that was added since on my end there are no extra characters appearing ; maybe it's a CR/LF handling even though I have the autoCrLF setting enabled in git...
I've removed the line altogether.

@costin costin merged commit 6cfe130 into elastic:master Apr 17, 2020
@costin costin deleted the sql/streamline-leafaggs branch April 17, 2020 12:03
costin added a commit that referenced this pull request Apr 17, 2020
Avoid repetition of the aggregation builder setup

Relates #55241

(cherry picked from commit 6cfe130)
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.

5 participants