Skip to content

Conversation

@viirya
Copy link
Member

@viirya viirya commented Apr 14, 2016

What changes were proposed in this pull request?

JIRA: https://issues.apache.org/jira/browse/SPARK-14627

Every time we call TypedAggregateExpression.update method, we call encoder's shift method. As shift method will copy encoder and underlying BoundReference, we should prepare the shifted encoder in advance, instead of calling shift method every time.

BTW, we can also slightly improve encoder's shift method to return itself when shift delta is zero.

How was this patch tested?

Existing tests.

@SparkQA
Copy link

SparkQA commented Apr 14, 2016

Test build #55804 has finished for PR 12387 at commit 05875ba.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@viirya
Copy link
Member Author

viirya commented Apr 14, 2016

A minor change. Should be straightforward. cc @marmbrus @cloud-fan @rxin

@cloud-fan
Copy link
Contributor

Thanks for working on it! But unfortunately, we are rewriting TypedAggregateExpression to make it use expressions, see #12067, i.e. there will be no shifting stuff anymore, do you mind closing this PR? thanks.

@viirya
Copy link
Member Author

viirya commented Apr 14, 2016

ok. close it now.

@viirya viirya closed this Apr 14, 2016
@viirya
Copy link
Member Author

viirya commented Apr 14, 2016

Ah. But the change in encoder should not related to #12067, right?

@viirya viirya reopened this Apr 14, 2016
@viirya viirya changed the title [SPARK-14627][SQL] Avoid shilfting encoder many times in TypedAggregateExpression [SPARK-14627][SQL] Avoid shilfting encoder when delta is zero Apr 14, 2016
@cloud-fan
Copy link
Contributor

But shift in only used in TypedAggregateExpression ...

@viirya
Copy link
Member Author

viirya commented Apr 14, 2016

ha. got it.

@viirya viirya closed this Apr 14, 2016
@viirya viirya deleted the skip-copy-bencoder branch December 27, 2023 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants