Skip to content

[FLINK-2725] Max/Min/Sum Aggregation of mutable types#1191

Closed
greghogan wants to merge 1 commit intoapache:masterfrom
greghogan:2725_max_min_sum_aggregation_of_mutable_types
Closed

[FLINK-2725] Max/Min/Sum Aggregation of mutable types#1191
greghogan wants to merge 1 commit intoapache:masterfrom
greghogan:2725_max_min_sum_aggregation_of_mutable_types

Conversation

@greghogan
Copy link
Contributor

No description provided.

@StephanEwen
Copy link
Contributor

Looks like a solid addition.

Out of curiosity: Have you made any experiments or microbenchmarks about how this impacts performance?

@greghogan
Copy link
Contributor Author

@StephanEwen, I have not yet created or run any microbenchmarks. Are you thinking of comparing immutable aggregators with and without this patch or comparing mutable and immutable aggregators?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the purpose of these statics that users's don't have to instantiate them themselves?
I'm just wondering because IntelliJ is marking some of them as unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, these follow the form of BasicTypeInfo (which, admittedly, are much longer instantiations). These are used extensively in ValueCollectionDataSets for creation of TupleTypeInfo and which mirrors CollectionDataSets which uses the BasicTypeInfo statics for the same purpose.

I am surprised IntelliJ is marking public members as unused.

@fhueske
Copy link
Contributor

fhueske commented Oct 16, 2015

Thanks for the PR!
I will merge it with the next batch.

@asfgit asfgit closed this in da248b1 Oct 19, 2015
s1ck pushed a commit to s1ck/flink that referenced this pull request Oct 20, 2015
cfmcgrady pushed a commit to cfmcgrady/flink that referenced this pull request Oct 23, 2015
lofifnc pushed a commit to lofifnc/flink that referenced this pull request Oct 23, 2015
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.

4 participants