Skip to content

Conversation

@KevinZwx
Copy link
Contributor

What changes were proposed in this pull request?

Remove the non-negative checks of window start time to make window support negative start time, and add a check to guarantee the absolute value of start time is less than slide duration.

How was this patch tested?

New unit tests.

@KevinZwx
Copy link
Contributor Author

KevinZwx commented Aug 11, 2017

@zsxwing @brkyvz Can you review this patch?

@brkyvz
Copy link
Contributor

brkyvz commented Aug 11, 2017

ok to test

}
}

test("SPARK-21590: Start time works with negative values and return microseconds") {
Copy link
Contributor

Choose a reason for hiding this comment

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

could you also add a DataFrame test for this with a negative value? then I'll feel a lot more comfortable that we don't have to have the start offset as a positive number in the window calculation.
Look for DataFrameTimeWindowingSuite.scala

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah thanks, I added some tests in DataFrameTimeWindowingSuite

@brkyvz
Copy link
Contributor

brkyvz commented Aug 11, 2017

test this please

@KevinZwx
Copy link
Contributor Author

KevinZwx commented Sep 6, 2017

test this please

@SparkQA
Copy link

SparkQA commented Apr 23, 2018

Test build #4154 has finished for PR 18903 at commit 07e98e7.

  • This patch fails PySpark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 13, 2018

Test build #4175 has finished for PR 18903 at commit 07e98e7.

  • This patch fails PySpark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented May 14, 2018

Test build #4178 has finished for PR 18903 at commit 07e98e7.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@kiszk
Copy link
Member

kiszk commented May 15, 2018

retest this please

@HyukjinKwon
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Jul 16, 2018

Test build #93048 has finished for PR 18903 at commit 07e98e7.

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

@srowen
Copy link
Member

srowen commented Jul 17, 2018

Merged to master

@asfgit asfgit closed this in 7688ce8 Jul 17, 2018
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.

6 participants