Skip to content

Conversation

@hvanhovell
Copy link
Contributor

What changes were proposed in this pull request?

The logic for LEAD/LAG processing is more complex that it needs to be. This PR fixes that.

How was this patch tested?

Existing tests.

@hvanhovell
Copy link
Contributor Author

cc @yhuai

@SparkQA
Copy link

SparkQA commented Jul 27, 2016

Test build #62916 has finished for PR 14376 at commit 427c179.

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

target,
ordinal,
functions,
functions.map(_.asInstanceOf[OffsetWindowFunction]),
Copy link
Contributor

Choose a reason for hiding this comment

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

leave a comment here about why it is safe to blindly cast?

@SparkQA
Copy link

SparkQA commented Jul 28, 2016

Test build #62967 has finished for PR 14376 at commit cecbd8b.

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

*/
private[execution] final class OffsetWindowFunctionFrame(
target: MutableRow,
ordinal: Int,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are changing this file, can you add comment to explain what is ordinal (with an example)?

Copy link
Contributor Author

@hvanhovell hvanhovell Aug 8, 2016

Choose a reason for hiding this comment

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

Done. In the class doc.

@SparkQA
Copy link

SparkQA commented Aug 8, 2016

Test build #63381 has finished for PR 14376 at commit 7e78b18.

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

@yhuai
Copy link
Contributor

yhuai commented Aug 8, 2016

Merging to master.

@asfgit asfgit closed this in df10658 Aug 8, 2016
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.

4 participants