Skip to content

Conversation

@x1-
Copy link
Contributor

@x1- x1- commented Aug 20, 2015

abstract

Through casting Timestamp to Long and Long to Timestamp, we should use same time unit.
I aligned milliseconds.

issue

@JoshRosen
Copy link
Contributor

Jenkins, this is ok to test.

@SparkQA
Copy link

SparkQA commented Aug 20, 2015

Test build #41321 has finished for PR 8339 at commit bd225ee.

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

@sarutak
Copy link
Member

sarutak commented Aug 21, 2015

retest this please.

@sarutak
Copy link
Member

sarutak commented Aug 21, 2015

@x1- The methods you changed are used from HiveQuerySuite so you should also modify tests in that test suite.
You can see which test cases you should modify in https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/41321/

@x1-
Copy link
Contributor Author

x1- commented Aug 21, 2015

@sarutak Thank you very much !
Now I modify tests, so later send PR again.

@sarutak
Copy link
Member

sarutak commented Aug 21, 2015

I noticed the precision was changed by #7283 .

@rxin @marmbrus @davies do you think this change is reasonable?

@SparkQA
Copy link

SparkQA commented Aug 21, 2015

Test build #41343 has finished for PR 8339 at commit bd225ee.

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

@x1-
Copy link
Contributor Author

x1- commented Aug 21, 2015

@sarutak
I tried casting Timestamp to Long and Long to Timestamp using Hive below.

hive> SELECT CAST(CAST(1.2 AS TIMESTAMP) AS DOUBLE);
OK
1.2
Time taken: 3.637 seconds, Fetched: 1 row(s)

hive> SELECT CAST(CAST(1200 AS TIMESTAMP) AS INT);
OK
1

This mean that Hive has contradiction in converting Timestamp to Int, and then Int to Timestamp.
Because of spark has different dialect, it is better way closing this PR, I think.

@x1-
Copy link
Contributor Author

x1- commented Aug 21, 2015

So, now I close this.

@x1- x1- closed this Aug 21, 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.

4 participants