[SPARK-5123][SQL] Expose only one version of the data type APIs (i.e. remove Java-specific APIs) #3925

rxin · 2015-01-07T05:49:20Z

Having two versions of the data type APIs (one for Java, one for Scala) requires downstream libraries to also have two versions of the APIs if the library wants to support both Java and Scala. I took a look at the Scala version of the data type APIs - it can actually work out pretty well for Java out of the box.

As part of the PR, I created a sql.types package and moved all type definitions there. I then removed the Java specific data type API along with a lot of the conversion code.

…ve the Java-specific API).

SparkQA · 2015-01-07T05:52:35Z

Test build #25141 has started for PR 3925 at commit 8eb5dc9.

This patch merges cleanly.

SparkQA · 2015-01-07T05:53:28Z

Test build #25141 has finished for PR 3925 at commit 8eb5dc9.

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

AmplabJenkins · 2015-01-07T05:53:29Z

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

pwendell · 2015-01-07T05:57:04Z

Is this something where we need to make sure to include upgrade details in the release notes?

rxin · 2015-01-07T05:59:57Z

Yes - is there a way to label these things in JIRA?

SparkQA · 2015-01-07T06:02:37Z

Test build #25142 has started for PR 3925 at commit 4674585.

This patch merges cleanly.

rxin · 2015-01-07T06:04:42Z

cc @marmbrus, @yhuai for SQL changes, and @mengxr, @jkbradley for MLlib changes ...

SparkQA · 2015-01-07T06:05:47Z

Test build #25142 has finished for PR 3925 at commit 4674585.

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

AmplabJenkins · 2015-01-07T06:05:48Z

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

SparkQA · 2015-01-07T06:17:38Z

Test build #25145 has started for PR 3925 at commit a2bb038.

This patch merges cleanly.

SparkQA · 2015-01-07T07:39:10Z

Test build #25145 has finished for PR 3925 at commit a2bb038.

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

AmplabJenkins · 2015-01-07T07:39:14Z

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

chenghao-intel · 2015-01-07T14:56:05Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/DataTypeConversions.scala

+      }
+    } else {
+      // ISO8601 with GMT insert
+      val ISO8601GMT: SimpleDateFormat = new SimpleDateFormat( "yyyy-MM-dd'T'HH:mm:ss.SSSz" )


Make ISO8601GMT this as thread local? or leave a TODO for future improvement.

What do you mean? I don't think I changed this. Simply copied it from a file that was deleted.

chenghao-intel · 2015-01-07T15:26:41Z

That's a very cool idea to make a unified DataType API for jvm-based language. Less code maintenance will be made!

yhuai · 2015-01-07T18:11:46Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/dataTypes.scala

+/**
+ * :: DeveloperApi ::
+ *
+ * The data type representing `NULL` values. Please use the singleton [[DataTypes.NullType]].


Are scala users expected to use DataTypes?

Sure why not ....

yhuai · 2015-01-07T18:55:05Z

SQL changes look good to me.

… into sql.types package.

SparkQA · 2015-01-07T19:22:37Z

Test build #25169 has started for PR 3925 at commit a1c1864.

This patch merges cleanly.

SparkQA · 2015-01-07T19:32:37Z

Test build #25170 has started for PR 3925 at commit 0cc1a5d.

This patch merges cleanly.

SparkQA · 2015-01-07T20:42:57Z

Test build #25169 has finished for PR 3925 at commit a1c1864.

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

AmplabJenkins · 2015-01-07T20:43:01Z

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

SparkQA · 2015-01-07T20:58:14Z

Test build #25170 has finished for PR 3925 at commit 0cc1a5d.

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

AmplabJenkins · 2015-01-07T20:58:18Z

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

SparkQA · 2015-01-07T21:42:38Z

Test build #25173 has started for PR 3925 at commit 5b77c6a.

This patch merges cleanly.

SparkQA · 2015-01-07T23:15:49Z

Test build #25173 has finished for PR 3925 at commit 5b77c6a.

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

AmplabJenkins · 2015-01-07T23:15:53Z

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

jkbradley · 2015-01-07T23:27:07Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/dataTypes.scala

+ * // nonExisting: StructField = null
+ *
+ * // Extract multiple StructFields. Field names are provided in a set.
+ * // A StructType object will be returned.


Should the "Preserve the original order of fields." comment in the apply() method be moved to the doc?

SparkQA · 2015-01-08T00:02:35Z

Test build #25184 has started for PR 3925 at commit c130913.

This patch merges cleanly.

jkbradley · 2015-01-08T00:09:49Z

@rxin MLlib changes look fine to me, and FWIW the other parts did too. My only remaining comments are ordering imports, but I'll leave those out for now. LGTM

SparkQA · 2015-01-08T01:41:35Z

Test build #25184 has finished for PR 3925 at commit c130913.

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

AmplabJenkins · 2015-01-08T01:41:39Z

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

SparkQA · 2015-01-08T02:17:36Z

Test build #25193 has started for PR 3925 at commit e98a7c0.

This patch merges cleanly.

SparkQA · 2015-01-08T03:55:21Z

Test build #25193 has finished for PR 3925 at commit e98a7c0.

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

AmplabJenkins · 2015-01-08T03:55:25Z

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

marmbrus · 2015-01-08T19:49:46Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveInspectorSuite.scala

@@ -93,7 +92,6 @@ class HiveInspectorSuite extends FunSuite with HiveInspectors {
  val row = data.map(_.eval(null))
  val dataTypes = data.map(_.dataType)

-  import scala.collection.JavaConversions._


Most of the code in Spark SQL uses JavaConversions... I don't think performance is an issue here or anything.

…ces.

SparkQA · 2015-01-08T20:47:30Z

Test build #25260 has started for PR 3925 at commit 603919f.

This patch merges cleanly.

SparkQA · 2015-01-08T22:28:25Z

Test build #25260 has finished for PR 3925 at commit 603919f.

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

AmplabJenkins · 2015-01-08T22:28:28Z

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

rxin · 2015-01-08T23:07:45Z

See #3958

Having two versions of the data type APIs (one for Java, one for Scala) requires downstream libraries to also have two versions of the APIs if the library wants to support both Java and Scala. I took a look at the Scala version of the data type APIs - it can actually work out pretty well for Java out of the box. As part of the PR, I created a sql.types package and moved all type definitions there. I then removed the Java specific data type API along with a lot of the conversion code. This subsumes #3925 Author: Reynold Xin <rxin@databricks.com> Closes #3958 from rxin/SPARK-5123-datatype-2 and squashes the following commits: 66505cc [Reynold Xin] [SPARK-5123] Expose only one version of the data type APIs (i.e. remove the Java-specific API).

scwf · 2015-01-18T11:26:45Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/DataTypes.java

@@ -15,77 +15,74 @@
 * limitations under the License.
 */

-package org.apache.spark.sql.api.java;
+package org.apache.spark.sql.types;


Since this is a java source file, should we move it to sql/catalyst/src/main/java ?

Follow up of #3925 /cc rxin Author: scwf <wangfei1@huawei.com> Closes #4095 from scwf/sql-doc and squashes the following commits: 97e311b [scwf] update sql doc since now expose only one version of the data type APIs

[SPARK-5123] Expose only one version of the data type APIs (i.e. remo…

8eb5dc9

…ve the Java-specific API).

Updated inline doc.

a523c35

Style fix.

4674585

Fix compilation error in HiveContext.

a2bb038

chenghao-intel reviewed Jan 7, 2015
View reviewed changes

yhuai reviewed Jan 7, 2015
View reviewed changes

Made StructType extend Seq[StructField], and moved SQLUserDefinedType…

a1c1864

… into sql.types package.

Renamed non-compilation code.

0cc1a5d

Fix schema parsing test.

5b77c6a

jkbradley reviewed Jan 7, 2015
View reviewed changes

Mima

c130913

code review, minor updates, and rearrange imports.

e98a7c0

marmbrus reviewed Jan 8, 2015
View reviewed changes

Added methods to maintain binary compatibility for external data sour…

603919f

…ces.

rxin mentioned this pull request Jan 8, 2015

[SPARK-5123][SQL] Reconcile Java/Scala API for data types. #3958

Closed

rxin closed this Jan 8, 2015

scwf mentioned this pull request Jan 18, 2015

[SQL][Minor] Update sql doc according to data type APIs changes #4095

Closed

scwf reviewed Jan 18, 2015
View reviewed changes

[SPARK-5123][SQL] Expose only one version of the data type APIs (i.e. remove Java-specific APIs) #3925

[SPARK-5123][SQL] Expose only one version of the data type APIs (i.e. remove Java-specific APIs) #3925

Conversation

rxin commented Jan 7, 2015

SparkQA commented Jan 7, 2015

SparkQA commented Jan 7, 2015

AmplabJenkins commented Jan 7, 2015

pwendell commented Jan 7, 2015

rxin commented Jan 7, 2015

SparkQA commented Jan 7, 2015

rxin commented Jan 7, 2015

SparkQA commented Jan 7, 2015

AmplabJenkins commented Jan 7, 2015

SparkQA commented Jan 7, 2015

SparkQA commented Jan 7, 2015

AmplabJenkins commented Jan 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenghao-intel commented Jan 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yhuai commented Jan 7, 2015

SparkQA commented Jan 7, 2015

SparkQA commented Jan 7, 2015

SparkQA commented Jan 7, 2015

AmplabJenkins commented Jan 7, 2015

SparkQA commented Jan 7, 2015

AmplabJenkins commented Jan 7, 2015

SparkQA commented Jan 7, 2015

SparkQA commented Jan 7, 2015

AmplabJenkins commented Jan 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Jan 8, 2015

jkbradley commented Jan 8, 2015

SparkQA commented Jan 8, 2015

AmplabJenkins commented Jan 8, 2015

SparkQA commented Jan 8, 2015

SparkQA commented Jan 8, 2015

AmplabJenkins commented Jan 8, 2015

Choose a reason for hiding this comment

SparkQA commented Jan 8, 2015

SparkQA commented Jan 8, 2015

AmplabJenkins commented Jan 8, 2015

rxin commented Jan 8, 2015

Choose a reason for hiding this comment