Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-4205][SQL] Timestamp and Date with comparisons #3158

Closed
wants to merge 17 commits into from
Closed

[SPARK-4205][SQL] Timestamp and Date with comparisons #3158

wants to merge 17 commits into from

Conversation

culler
Copy link

@culler culler commented Nov 7, 2014

This PR adds new RichDate and RichTimestamp classes that provide comparison operators, allowing them to be used in DSL expressions, and initializers which accept string representations of dates or times.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@culler
Copy link
Author

culler commented Nov 7, 2014

@liangcheng and @rxin, I am reopening pull request #3066 as #3158 so it can be based on a current commit of the spark source. I messed up #3066 by trying to rebase it after a new test file was added which required minor changes to compile. Sorry for any confusion this causes.

@marmbrus
Copy link
Contributor

marmbrus commented Nov 7, 2014

Thanks for working on this. Is a pretty big change so I'll review it next week after we get some critical fixes in for 1.2.

@culler
Copy link
Author

culler commented Nov 7, 2014

Thanks @marmbrus. That sounds good. If it isn't too much trouble, I'd appreciate seeing what jenkins has to say before you review it next week.

@marmbrus
Copy link
Contributor

marmbrus commented Nov 7, 2014

ok to test

@SparkQA
Copy link

SparkQA commented Nov 7, 2014

Test build #23075 has started for PR 3158 at commit ef5e4a4.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 7, 2014

Test build #23075 has finished for PR 3158 at commit ef5e4a4.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class LhsLiteral(x: Any)
    • final class MutableDate extends MutableValue
    • final class MutableTimestamp extends MutableValue
    • class RichDate(milliseconds: Long) extends Date(milliseconds)
    • class RichTimestamp(milliseconds: Long) extends Timestamp(milliseconds)

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23075/
Test FAILed.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

1 similar comment
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@culler
Copy link
Author

culler commented Nov 8, 2014

Oh well. Some new tests were added Friday to sql/core/src/test/scala/org/apache/spark/sql/parquet/ParquetQuerySuite.scala which use the construction assert(X === Y). Except for adding a few tests for the new features in this PR, all of the changes to the test files consisted of replacing
assert(X === Y) by assert(convertToEqualizer(X).===(Y)), which is what the scalatest documentation recommends in cases where an === operator is already defined for X. In this situation the === operators are being added by the implicit conversion which enables DSL expressions to begin with a literal. The affected tests are the ones which import these implicit conversions.

@SparkQA
Copy link

SparkQA commented Nov 8, 2014

Test build #23104 has started for PR 3158 at commit b61540e.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 8, 2014

Test build #23104 has finished for PR 3158 at commit b61540e.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class LhsLiteral(x: Any)
    • final class MutableDate extends MutableValue
    • final class MutableTimestamp extends MutableValue
    • class RichDate(milliseconds: Long) extends Date(milliseconds)
    • class RichTimestamp(milliseconds: Long) extends Timestamp(milliseconds)

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23104/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Nov 8, 2014

Test build #23105 has started for PR 3158 at commit e1ba56e.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 8, 2014

Test build #23106 has started for PR 3158 at commit 97d00a8.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 8, 2014

Test build #23105 has finished for PR 3158 at commit e1ba56e.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class LhsLiteral(x: Any)
    • final class MutableDate extends MutableValue
    • final class MutableTimestamp extends MutableValue
    • class RichDate(milliseconds: Long) extends Date(milliseconds)
    • class RichTimestamp(milliseconds: Long) extends Timestamp(milliseconds)

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23105/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Nov 8, 2014

Test build #23106 has finished for PR 3158 at commit 97d00a8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class LhsLiteral(x: Any)
    • final class MutableDate extends MutableValue
    • final class MutableTimestamp extends MutableValue
    • class RichDate(milliseconds: Long) extends Date(milliseconds)
    • class RichTimestamp(milliseconds: Long) extends Timestamp(milliseconds)

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23106/
Test PASSed.

@SparkQA
Copy link

SparkQA commented Nov 9, 2014

Test build #23123 has started for PR 3158 at commit c5fb299.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 9, 2014

Test build #23123 has finished for PR 3158 at commit c5fb299.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class LhsLiteral(x: Any)
    • final class MutableDate extends MutableValue
    • final class MutableTimestamp extends MutableValue
    • class RichDate(milliseconds: Long) extends Date(milliseconds)
    • class RichTimestamp(milliseconds: Long) extends Timestamp(milliseconds)

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23123/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Nov 10, 2014

Test build #23130 has started for PR 3158 at commit 0af7a3d.

  • This patch merges cleanly.

allowing them to be used in DSL expressions.  These classes provide
initializers which accept string representations of dates or times.
They are renamed as Date and Timestamp when the members of an
SQLContext are in scope.
with a literal, e.g. 0 < 'x .
These conversions expose a conflict with the scalatest === operator
if assert(X === Y) is used when the conversions are in scope.  To
fix this, several tests are modified, as recommended in the scalatest
documentation, by making the change:
assert(X === Y) --> assert(convertToEqualizer(X).===(Y))
a test which would have detected the problem.
available after importing the members of an SQLContext.
@SparkQA
Copy link

SparkQA commented Nov 11, 2014

Test build #23171 has finished for PR 3158 at commit dabcaf2.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class LhsLiteral(x: Any)
    • class RichDate(milliseconds: Long) extends Date(milliseconds)
    • class RichTimestamp(milliseconds: Long) extends Timestamp(milliseconds)

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23171/
Test FAILed.

@culler culler changed the title [SPARK-4205][SQL] Timestamp and Date with comparisons / DSL literals [SPARK-4205][SQL] Timestamp and Date with comparisons Nov 11, 2014
@SparkQA
Copy link

SparkQA commented Nov 11, 2014

Test build #23193 has started for PR 3158 at commit f917e06.

  • This patch merges cleanly.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23192/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Nov 11, 2014

Test build #23193 has finished for PR 3158 at commit f917e06.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class RichDate(milliseconds: Long) extends Date(milliseconds)
    • class RichTimestamp(milliseconds: Long) extends Timestamp(milliseconds)

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23193/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Nov 11, 2014

Test build #23195 has started for PR 3158 at commit 4c32c04.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 11, 2014

Test build #23195 has finished for PR 3158 at commit 4c32c04.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class RichDate(milliseconds: Long) extends Date(milliseconds)
    • class RichTimestamp(milliseconds: Long) extends Timestamp(milliseconds)

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23195/
Test PASSed.

@culler
Copy link
Author

culler commented Nov 11, 2014

OK @marmbrus and @liancheng, this PR is now fit and slim. The LhsLiterals have been removed and will appear in a new PR. This one passes all tests and I think it is ready for review, finally.

谢谢, @liancheng, for your patience, thoughtful comments and careful reading.

@culler
Copy link
Author

culler commented Nov 11, 2014

Thanks!

On Mon, Nov 10, 2014 at 6:17 PM, Michael Armbrust notifications@github.com
wrote:

Quick note: you don't need a fork per PR. You can have multiple branches
in a single fork, each of them with a corresponding PR.


Reply to this email directly or view it on GitHub
#3158 (comment).

@culler
Copy link
Author

culler commented Nov 15, 2014

Maybe next week will be better. The PR is now much smaller, with the
changes to test files removed and the DSL part moved into PR #3210.

On Fri, Nov 7, 2014 at 3:02 PM, Michael Armbrust notifications@github.com
wrote:

Thanks for working on this. Is a pretty big change so I'll review it next
week after we get some critical fixes in for 1.2.


Reply to this email directly or view it on GitHub
#3158 (comment).

@SparkQA
Copy link

SparkQA commented Dec 9, 2014

Test build #24259 has started for PR 3158 at commit 4cfb864.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Dec 9, 2014

Test build #24259 has finished for PR 3158 at commit 4cfb864.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class RichDate(milliseconds: Long) extends Date(milliseconds)
    • class RichTimestamp(milliseconds: Long) extends Timestamp(milliseconds)

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24259/
Test PASSed.

@SparkQA
Copy link

SparkQA commented Jan 6, 2015

Test build #25108 has started for PR 3158 at commit dd39a89.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 6, 2015

Test build #25110 has started for PR 3158 at commit 31f0d7e.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 6, 2015

Test build #25108 has finished for PR 3158 at commit dd39a89.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25108/
Test PASSed.

@SparkQA
Copy link

SparkQA commented Jan 6, 2015

Test build #25110 has finished for PR 3158 at commit 31f0d7e.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25110/
Test PASSed.

@asfgit asfgit closed this in ffe8cc9 Apr 3, 2015
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.

5 participants