[SPARK-8307] [SQL] improve timestamp from parquet #6759

davies · 2015-06-11T07:40:55Z

This PR change to convert julian day to unix timestamp directly (without Calendar and Timestamp).

adrian-wang · 2015-06-11T07:44:20Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala

@@ -26,7 +26,7 @@ import org.apache.spark.sql.catalyst.expressions.Cast
 /**
 * Helper function to convert between Int value of days since 1970-01-01 and java.sql.Date


java doc here

SparkQA · 2015-06-11T07:44:55Z

Test build #34675 has finished for PR 6759 at commit ea196d4.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

rxin · 2015-06-11T07:47:07Z

sql/core/src/main/scala/org/apache/spark/sql/parquet/ParquetConverter.scala


-  def convertToTimestamp(value: Binary): Timestamp = {
+  def convertToTimestamp(value: Binary): Long = {
    val nt = NanoTime.fromBinary(value)


we should just get rid of NanoTime ...

SparkQA · 2015-06-11T23:19:19Z

Test build #34724 has finished for PR 6759 at commit 361fd62.

This patch fails Scala style tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2015-06-11T23:37:34Z

Test build #34726 has finished for PR 6759 at commit 6e445e6.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

Conflicts: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala sql/core/src/main/scala/org/apache/spark/sql/parquet/ParquetConverter.scala

SparkQA · 2015-06-12T00:01:29Z

Test build #34728 has finished for PR 6759 at commit a3171b8.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-06-12T00:37:22Z

Test build #898 has finished for PR 6759 at commit a3171b8.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

Conflicts: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/CatalystTypeConverters.scala sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/DateTimeUtilsSuite.scala sql/core/src/main/scala/org/apache/spark/sql/jdbc/JDBCRDD.scala sql/core/src/main/scala/org/apache/spark/sql/parquet/ParquetTableSupport.scala sql/core/src/test/scala/org/apache/spark/sql/parquet/ParquetIOSuite.scala

SparkQA · 2015-06-13T21:35:45Z

Test build #34841 has finished for PR 6759 at commit c834108.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-06-13T21:41:16Z

Test build #34842 has finished for PR 6759 at commit 212143b.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-06-13T23:58:24Z

Test build #34849 has finished for PR 6759 at commit 8ace611.

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

SparkQA · 2015-06-14T01:28:22Z

Test build #34858 has finished for PR 6759 at commit 2f2e48c.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-06-14T08:32:59Z

Test build #911 has finished for PR 6759 at commit 2f2e48c.

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

SparkQA · 2015-06-14T08:36:21Z

Test build #912 has finished for PR 6759 at commit 2f2e48c.

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

SparkQA · 2015-06-14T08:37:01Z

Test build #913 has finished for PR 6759 at commit 2f2e48c.

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

SparkQA · 2015-06-15T03:50:55Z

Test build #34899 has finished for PR 6759 at commit 602b969.

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

davies · 2015-06-17T05:10:43Z

@rxin This is ready to review

JoshRosen · 2015-06-19T18:42:43Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala

+   * and nanoseconds in a day
+   */
+  def fromJulianDay(day: Int, nanoseconds: Long): Long = {
+    // use integer to avoid rounding errors


It looks like we're using longs here to avoid the rounding errors, not integers, so maybe we should update this comment.

SparkQA · 2015-06-19T19:28:38Z

Test build #35296 has finished for PR 6759 at commit bfc437c.

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

JoshRosen · 2015-06-19T19:48:17Z

sql/core/src/main/scala/org/apache/spark/sql/parquet/ParquetConverter.scala

+    DateTimeUtils.fromJulianDay(julianDay, timeOfDayNanos)
+  }
+
+  def convertFromTimestamp(ts: Long): Binary = {


Similarly, I think we should add a comment here as well (it can be the inverse of my other suggested Scaladoc comment).

JoshRosen · 2015-06-19T20:26:53Z

At a high level, this looks like it's in really good shape. I might have more comments in a bit; have to step out to handle some calls now, but I'll loop back later in the afternoon to finish looking this over.

SparkQA · 2015-06-19T21:19:19Z

Test build #35321 has finished for PR 6759 at commit 4891efb.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

JoshRosen · 2015-06-19T21:30:26Z

sql/core/src/main/scala/org/apache/spark/sql/parquet/ParquetTableSupport.scala

@@ -313,10 +314,16 @@ private[parquet] class RowWriteSupport extends WriteSupport[InternalRow] with Lo
    writer.addBinary(Binary.fromByteArray(scratchBytes, 0, numBytes))
  }

+  // array used to write Timestamp as Int96 (fixed-length binary)
+  private val int96buf = new Array[Byte](12)


This assumes that RowWriteSupport instances are thread-safe; just want to check that that assumption is true.

This could also be private[this].

Actually, I guess this is fine since we use the same pattern with scratchBytes for writeDecimal in the preceding lines.

SparkQA · 2015-06-19T23:39:17Z

Test build #35338 has finished for PR 6759 at commit 8e2d56f.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-06-20T00:40:16Z

Test build #35332 has finished for PR 6759 at commit 634b9f5.

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

SparkQA · 2015-06-20T02:10:22Z

Test build #941 has finished for PR 6759 at commit 8e2d56f.

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

SparkQA · 2015-06-20T03:35:52Z

Test build #942 has finished for PR 6759 at commit 8e2d56f.

This patch passes all tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2015-06-20T23:11:57Z

Test build #945 has finished for PR 6759 at commit 8e2d56f.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2015-06-21T00:58:30Z

Test build #948 has finished for PR 6759 at commit 8e2d56f.

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

Conflicts: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala

SparkQA · 2015-06-22T22:40:41Z

Test build #35481 has finished for PR 6759 at commit b0e4cad.

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

Conflicts: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/UnsafeRowConverterSuite.scala

davies · 2015-06-22T22:44:30Z

@JoshRosen Is this ready to go?

JoshRosen · 2015-06-22T22:50:18Z

@davies, yeah, I think this should be good to go; looks like all of my comments have been addressed.

SparkQA · 2015-06-23T00:40:14Z

Test build #35489 has finished for PR 6759 at commit 849e301.

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

davies · 2015-06-23T01:03:39Z

The last failure is not related, I'd merge this into master.

This PR change to convert julian day to unix timestamp directly (without Calendar and Timestamp). cc adrian-wang rxin Author: Davies Liu <davies@databricks.com> Closes apache#6759 from davies/improve_ts and squashes the following commits: 849e301 [Davies Liu] Merge branch 'master' of github.com:apache/spark into improve_ts b0e4cad [Davies Liu] Merge branch 'master' of github.com:apache/spark into improve_ts 8e2d56f [Davies Liu] address comments 634b9f5 [Davies Liu] fix mima 4891efb [Davies Liu] address comment bfc437c [Davies Liu] fix build ae5979c [Davies Liu] Merge branch 'master' of github.com:apache/spark into improve_ts 602b969 [Davies Liu] remove jodd 2f2e48c [Davies Liu] fix test 8ace611 [Davies Liu] fix mima 212143b [Davies Liu] fix mina c834108 [Davies Liu] Merge branch 'master' of github.com:apache/spark into improve_ts a3171b8 [Davies Liu] Merge branch 'master' of github.com:apache/spark into improve_ts 5233974 [Davies Liu] fix scala style 361fd62 [Davies Liu] address comments ea196d4 [Davies Liu] improve timestamp from parquet

improve timestamp from parquet

ea196d4

davies mentioned this pull request Jun 11, 2015

[SPARK-8189] [SQL] use Long for TimestampType in SQL #6733

Closed

adrian-wang reviewed Jun 11, 2015
View reviewed changes

rxin reviewed Jun 11, 2015
View reviewed changes

address comments

361fd62

fix scala style

5233974

Merge branch 'master' of github.com:apache/spark into improve_ts

a3171b8

Conflicts: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala sql/core/src/main/scala/org/apache/spark/sql/parquet/ParquetConverter.scala

davies force-pushed the improve_ts branch from 6e445e6 to a3171b8 Compare June 11, 2015 23:43

adrian-wang mentioned this pull request Jun 12, 2015

[SPARK-8186] [SPARK-8187] [SQL] datetime function: date_add, date_sub #6782

Closed

Davies Liu added 2 commits June 13, 2015 14:19

fix mina

212143b

fix mima

8ace611

fix test

2f2e48c

remove jodd

602b969

JoshRosen reviewed Jun 19, 2015
View reviewed changes

address comment

4891efb

JoshRosen reviewed Jun 19, 2015
View reviewed changes

Davies Liu added 2 commits June 19, 2015 15:08

fix mima

634b9f5

address comments

8e2d56f

Merge branch 'master' of github.com:apache/spark into improve_ts

b0e4cad

Conflicts: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala

Merge branch 'master' of github.com:apache/spark into improve_ts

849e301

Conflicts: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/UnsafeRowConverterSuite.scala

asfgit closed this in 6b7f2ce Jun 23, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-8307] [SQL] improve timestamp from parquet #6759

[SPARK-8307] [SQL] improve timestamp from parquet #6759

davies commented Jun 11, 2015

adrian-wang Jun 11, 2015

SparkQA commented Jun 11, 2015

rxin Jun 11, 2015

SparkQA commented Jun 11, 2015

SparkQA commented Jun 11, 2015

SparkQA commented Jun 12, 2015

SparkQA commented Jun 12, 2015

SparkQA commented Jun 13, 2015

SparkQA commented Jun 13, 2015

SparkQA commented Jun 13, 2015

SparkQA commented Jun 14, 2015

SparkQA commented Jun 14, 2015

SparkQA commented Jun 14, 2015

SparkQA commented Jun 14, 2015

SparkQA commented Jun 15, 2015

davies commented Jun 17, 2015

JoshRosen Jun 19, 2015

SparkQA commented Jun 19, 2015

JoshRosen Jun 19, 2015

JoshRosen commented Jun 19, 2015

SparkQA commented Jun 19, 2015

JoshRosen Jun 19, 2015

JoshRosen Jun 19, 2015

SparkQA commented Jun 19, 2015

SparkQA commented Jun 20, 2015

SparkQA commented Jun 20, 2015

SparkQA commented Jun 20, 2015

SparkQA commented Jun 20, 2015

SparkQA commented Jun 21, 2015

SparkQA commented Jun 22, 2015

davies commented Jun 22, 2015

JoshRosen commented Jun 22, 2015

SparkQA commented Jun 23, 2015

davies commented Jun 23, 2015

		@@ -26,7 +26,7 @@ import org.apache.spark.sql.catalyst.expressions.Cast
		/**
		* Helper function to convert between Int value of days since 1970-01-01 and java.sql.Date

[SPARK-8307] [SQL] improve timestamp from parquet #6759

[SPARK-8307] [SQL] improve timestamp from parquet #6759

Conversation

davies commented Jun 11, 2015

adrian-wang Jun 11, 2015

Choose a reason for hiding this comment

SparkQA commented Jun 11, 2015

rxin Jun 11, 2015

Choose a reason for hiding this comment

SparkQA commented Jun 11, 2015

SparkQA commented Jun 11, 2015

SparkQA commented Jun 12, 2015

SparkQA commented Jun 12, 2015

SparkQA commented Jun 13, 2015

SparkQA commented Jun 13, 2015

SparkQA commented Jun 13, 2015

SparkQA commented Jun 14, 2015

SparkQA commented Jun 14, 2015

SparkQA commented Jun 14, 2015

SparkQA commented Jun 14, 2015

SparkQA commented Jun 15, 2015

davies commented Jun 17, 2015

JoshRosen Jun 19, 2015

Choose a reason for hiding this comment

SparkQA commented Jun 19, 2015

JoshRosen Jun 19, 2015

Choose a reason for hiding this comment

JoshRosen commented Jun 19, 2015

SparkQA commented Jun 19, 2015

JoshRosen Jun 19, 2015

Choose a reason for hiding this comment

JoshRosen Jun 19, 2015

Choose a reason for hiding this comment

SparkQA commented Jun 19, 2015

SparkQA commented Jun 20, 2015

SparkQA commented Jun 20, 2015

SparkQA commented Jun 20, 2015

SparkQA commented Jun 20, 2015

SparkQA commented Jun 21, 2015

SparkQA commented Jun 22, 2015

davies commented Jun 22, 2015

JoshRosen commented Jun 22, 2015

SparkQA commented Jun 23, 2015

davies commented Jun 23, 2015