You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by rick-ibm <gi...@git.apache.org> on 2015/10/05 18:58:39 UTC

[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

GitHub user rick-ibm opened a pull request:

    https://github.com/apache/spark/pull/8982

    [SPARK-10855] [SQL] Add a JDBC dialect for Apache Derby

    @marmbrus
    @rxin
    
    This patch adds a JdbcDialect class, which customizes the datatype mappings for Derby backends. The patch also adds unit tests for the new dialect, corresponding to the existing tests for other JDBC dialects.
    
    JDBCSuite runs cleanly for me with this patch. So does JDBCWriteSuite, although it produces noise as described here: https://issues.apache.org/jira/browse/SPARK-10890
    
    This patch is my original work, which I license to the ASF. I am a Derby contributor, so my ICLA is on file under SVN id "rhillegas": http://people.apache.org/committer-index.html
    
    Touches the following files:
    
    ---------------------------------
    
    org.apache.spark.sql.jdbc.JdbcDialects
    
    Adds a DerbyDialect.
    
    
    ---------------------------------
    
    org.apache.spark.sql.jdbc.JDBCSuite
    
    Adds unit tests for the new DerbyDialect.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/rick-ibm/spark b_10855

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/8982.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #8982
    
----
commit cf51be37e41097c8342bc94c174bc767ca9ee567
Author: Rick Hillegas <rh...@us.ibm.com>
Date:   2015-09-30T22:32:14Z

    SPARK-10855: Add a JDBC dialect for Apache Derby. Add unit tests for the new dialect.

commit 21decab9d8aa98364d7d6aa7c71e2082b4cd7d69
Author: Rick Hillegas <rh...@us.ibm.com>
Date:   2015-10-02T21:14:46Z

    SPARK-10855: Fix DECIMAL mapping for Derby JdbcDialect so that Spark can create Derby tables with DECIMAL columns.

commit ffb3fdef9f914ecef9333d727a9667511d994907
Author: Rick Hillegas <rh...@us.ibm.com>
Date:   2015-10-02T22:36:48Z

    SPARK-10855: For Derby JDBC dialect, only change the precision/scale of a DECIMAL if the max Derby precision is exceeded.

commit 11fdb0411c2413e8f13530960684a7023236b23d
Author: Rick Hillegas <rh...@us.ibm.com>
Date:   2015-10-05T16:15:13Z

    SPARK-10855: Correct an overly long line.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41439632
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
    @@ -464,5 +476,6 @@ class JDBCSuite extends SparkFunSuite with BeforeAndAfter with SharedSQLContext
         assert(Postgres.getTableExistsQuery(table) == limitQuery)
         assert(db2.getTableExistsQuery(table) == defaultQuery)
         assert(h2.getTableExistsQuery(table) == defaultQuery)
    +    assert(derby.getTableExistsQuery(table) == defaultQuery)
    --- End diff --
    
    use ===


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146914291
  
      [Test build #43478 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43478/consoleFull) for   PR 8982 at commit [`e6022ed`](https://github.com/apache/spark/commit/e6022ed8825f92751b3df4ab1efab9b60c6d8d3c).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rick-ibm <gi...@git.apache.org>.
Github user rick-ibm commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146943701
  
    My latest commit (d56897e) adjusts the Option usage per Reynold and Michael's advice. I believe that I have addressed all issues with this pull request. We can file a new JIRA if we want to harmonize the usage of Option in JdbcDialects.scala. Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146971870
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43481/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-145699761
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43250/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146942916
  
    Merged build started.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rick-ibm <gi...@git.apache.org>.
Github user rick-ibm commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41649765
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
    @@ -464,5 +476,6 @@ class JDBCSuite extends SparkFunSuite with BeforeAndAfter with SharedSQLContext
         assert(Postgres.getTableExistsQuery(table) == limitQuery)
         assert(db2.getTableExistsQuery(table) == defaultQuery)
         assert(h2.getTableExistsQuery(table) == defaultQuery)
    +    assert(derby.getTableExistsQuery(table) == defaultQuery)
    --- End diff --
    
    Thanks for the review comments, Reynold and Michael. Hopefully, my last commit (e6022ed) addresses your concerns:
    
    o I adjusted the indentation of a method signature, per Reynold's advice.
    
    o I simplified an "if" expression, also per Reynold's advice. However, I have a question about this change (see my comment above on the line in question).
    
    Thanks,
    -Rick


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-145699684
  
      [Test build #43250 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43250/console) for   PR 8982 at commit [`11fdb04`](https://github.com/apache/spark/commit/11fdb0411c2413e8f13530960684a7023236b23d).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rick-ibm <gi...@git.apache.org>.
Github user rick-ibm commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41649575
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
    @@ -287,3 +288,30 @@ case object MsSqlServerDialect extends JdbcDialect {
         case _ => None
       }
     }
    +
    +/**
    + * :: DeveloperApi ::
    + * Default Apache Derby dialect, mapping real on read
    + * and string/byte/short/boolean/decimal on write.
    + */
    +@DeveloperApi
    +case object DerbyDialect extends JdbcDialect {
    +  override def canHandle(url: String): Boolean = url.startsWith("jdbc:derby")
    +  override def getCatalystType(
    +      sqlType: Int, typeName: String, size: Int, md: MetadataBuilder): Option[DataType] = {
    +    if (sqlType == Types.REAL) Some(FloatType) else None
    --- End diff --
    
    I have changed the original code so that it matches the pattern followed by the other getCatalystType() overloads: The code returns a Some(FloatType) rather than an Option(FloatType). I am new to programming in Scala, but the former expression is the pattern which I see all over the code. However, Reynold recommended the latter expression. If Option(FloatType) is the correct value to return, then we should probably change all of the other getCatalystType() overloads to return Option(...) rather than Some(...). I would appreciate your guidance on this point. Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146727615
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43436/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146980480
  
    Thanks - I've merged this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146893399
  
      [Test build #43471 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43471/consoleFull) for   PR 8982 at commit [`989ce9b`](https://github.com/apache/spark/commit/989ce9bbf00ef222490d69f6172732572f769050).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146710467
  
      [Test build #43436 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43436/consoleFull) for   PR 8982 at commit [`f83d86c`](https://github.com/apache/spark/commit/f83d86ce01c3943595518503230e283b3062a44d).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146891794
  
    Merged build started.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146913813
  
     Merged build triggered.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146926932
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43471/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146926769
  
      [Test build #43471 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43471/console) for   PR 8982 at commit [`989ce9b`](https://github.com/apache/spark/commit/989ce9bbf00ef222490d69f6172732572f769050).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146948839
  
      [Test build #43478 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43478/console) for   PR 8982 at commit [`e6022ed`](https://github.com/apache/spark/commit/e6022ed8825f92751b3df4ab1efab9b60c6d8d3c).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rick-ibm <gi...@git.apache.org>.
Github user rick-ibm commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41650794
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
    @@ -287,3 +288,30 @@ case object MsSqlServerDialect extends JdbcDialect {
         case _ => None
       }
     }
    +
    +/**
    + * :: DeveloperApi ::
    + * Default Apache Derby dialect, mapping real on read
    + * and string/byte/short/boolean/decimal on write.
    + */
    +@DeveloperApi
    +case object DerbyDialect extends JdbcDialect {
    +  override def canHandle(url: String): Boolean = url.startsWith("jdbc:derby")
    +  override def getCatalystType(
    +      sqlType: Int, typeName: String, size: Int, md: MetadataBuilder): Option[DataType] = {
    +    if (sqlType == Types.REAL) Some(FloatType) else None
    --- End diff --
    
    Thanks for the quick response, Michael. Should I adjust the other getCatalystType() overloads to follow the Option(...) pattern? I could do that as part of this pull-request or I could open another JIRA if you think that is worth doing. Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41439667
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
    @@ -409,18 +409,22 @@ class JDBCSuite extends SparkFunSuite with BeforeAndAfter with SharedSQLContext
         assert(JdbcDialects.get("jdbc:postgresql://127.0.0.1/db") == PostgresDialect)
         assert(JdbcDialects.get("jdbc:db2://127.0.0.1/db") == DB2Dialect)
         assert(JdbcDialects.get("jdbc:sqlserver://127.0.0.1/db") == MsSqlServerDialect)
    +    assert(JdbcDialects.get("jdbc:derby:db") == DerbyDialect)
    --- End diff --
    
    can you change everything in this function to use ===


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41439501
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
    @@ -278,3 +279,32 @@ case object MsSqlServerDialect extends JdbcDialect {
         } else None
       }
     }
    +
    +/**
    + * :: DeveloperApi ::
    + * Default Apache Derby dialect, mapping real on read
    + * and string/byte/short/boolean/decimal on write.
    + */
    +@DeveloperApi
    +case object DerbyDialect extends JdbcDialect {
    +  override def canHandle(url: String): Boolean = url.startsWith("jdbc:derby")
    +  override def getCatalystType(
    +    sqlType: Int, typeName: String, size: Int, md: MetadataBuilder): Option[DataType] = {
    --- End diff --
    
    indent 4 space here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146727613
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/8982


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146971869
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-145674652
  
     Merged build triggered.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41558960
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
    @@ -464,5 +476,6 @@ class JDBCSuite extends SparkFunSuite with BeforeAndAfter with SharedSQLContext
         assert(Postgres.getTableExistsQuery(table) == limitQuery)
         assert(db2.getTableExistsQuery(table) == defaultQuery)
         assert(h2.getTableExistsQuery(table) == defaultQuery)
    +    assert(derby.getTableExistsQuery(table) == defaultQuery)
    --- End diff --
    
    Yes you are right. This no longer matters with the new version of ScalaTest. Just ignore my comments about ===. Please do fix the indents though. 
    
    LGTM otherwise.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146913843
  
    Merged build started.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146949035
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43478/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-145673472
  
    ;Jenkins, this is ok to test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41472441
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
    @@ -464,5 +476,6 @@ class JDBCSuite extends SparkFunSuite with BeforeAndAfter with SharedSQLContext
         assert(Postgres.getTableExistsQuery(table) == limitQuery)
         assert(db2.getTableExistsQuery(table) == defaultQuery)
         assert(h2.getTableExistsQuery(table) == defaultQuery)
    +    assert(derby.getTableExistsQuery(table) == defaultQuery)
    --- End diff --
    
    Does that actually matter anymore?  I thought it was macro magic.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41650090
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
    @@ -287,3 +288,30 @@ case object MsSqlServerDialect extends JdbcDialect {
         case _ => None
       }
     }
    +
    +/**
    + * :: DeveloperApi ::
    + * Default Apache Derby dialect, mapping real on read
    + * and string/byte/short/boolean/decimal on write.
    + */
    +@DeveloperApi
    +case object DerbyDialect extends JdbcDialect {
    +  override def canHandle(url: String): Boolean = url.startsWith("jdbc:derby")
    +  override def getCatalystType(
    +      sqlType: Int, typeName: String, size: Int, md: MetadataBuilder): Option[DataType] = {
    +    if (sqlType == Types.REAL) Some(FloatType) else None
    --- End diff --
    
    In this case (where you are clearly not passing null) they are equivalent.
    Option will return None in the case of null, which is why I prefer using
    that method of construction.
    On Oct 9, 2015 9:28 AM, "Rick Hillegas" <no...@github.com> wrote:
    
    > In sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
    > <https://github.com/apache/spark/pull/8982#discussion_r41649575>:
    >
    > > @@ -287,3 +288,30 @@ case object MsSqlServerDialect extends JdbcDialect {
    > >      case _ => None
    > >    }
    > >  }
    > > +
    > > +/**
    > > + * :: DeveloperApi ::
    > > + * Default Apache Derby dialect, mapping real on read
    > > + * and string/byte/short/boolean/decimal on write.
    > > + */
    > > +@DeveloperApi
    > > +case object DerbyDialect extends JdbcDialect {
    > > +  override def canHandle(url: String): Boolean = url.startsWith("jdbc:derby")
    > > +  override def getCatalystType(
    > > +      sqlType: Int, typeName: String, size: Int, md: MetadataBuilder): Option[DataType] = {
    > > +    if (sqlType == Types.REAL) Some(FloatType) else None
    >
    > I have changed the original code so that it matches the pattern followed
    > by the other getCatalystType() overloads: The code returns a
    > Some(FloatType) rather than an Option(FloatType). I am new to programming
    > in Scala, but the former expression is the pattern which I see all over the
    > code. However, Reynold recommended the latter expression. If
    > Option(FloatType) is the correct value to return, then we should probably
    > change all of the other getCatalystType() overloads to return Option(...)
    > rather than Some(...). I would appreciate your guidance on this point.
    > Thanks.
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/spark/pull/8982/files#r41649575>.
    >



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146942858
  
     Merged build triggered.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146949034
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146891761
  
     Merged build triggered.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146709463
  
     Merged build triggered.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rick-ibm <gi...@git.apache.org>.
Github user rick-ibm commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-145921778
  
    Tests seem to have passed cleanly. Any comments? Suggestions for improvements? Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146709482
  
    Merged build started.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-145676126
  
      [Test build #43250 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43250/consoleFull) for   PR 8982 at commit [`11fdb04`](https://github.com/apache/spark/commit/11fdb0411c2413e8f13530960684a7023236b23d).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rick-ibm <gi...@git.apache.org>.
Github user rick-ibm commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41649928
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
    @@ -287,3 +288,30 @@ case object MsSqlServerDialect extends JdbcDialect {
         case _ => None
       }
     }
    +
    +/**
    + * :: DeveloperApi ::
    + * Default Apache Derby dialect, mapping real on read
    + * and string/byte/short/boolean/decimal on write.
    + */
    +@DeveloperApi
    +case object DerbyDialect extends JdbcDialect {
    +  override def canHandle(url: String): Boolean = url.startsWith("jdbc:derby")
    +  override def getCatalystType(
    +      sqlType: Int, typeName: String, size: Int, md: MetadataBuilder): Option[DataType] = {
    +    if (sqlType == Types.REAL) Some(FloatType) else None
    --- End diff --
    
    
    
    Thanks for the review comments, Reynold and Michael. Hopefully, my last commit (e6022ed) addresses your concerns:
    
    o I adjusted the indentation of a method signature, per Reynold's advice.
    
    o I simplified an "if" expression, also per Reynold's advice. However, I have a question about this change (see my comment above on the line in question).
    
    Thanks,
    -Rick



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146945786
  
      [Test build #43481 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43481/consoleFull) for   PR 8982 at commit [`d56897e`](https://github.com/apache/spark/commit/d56897e0d3d5f0a2a007eeb3dda92d7f5a00d29c).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-145699758
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146727544
  
      [Test build #43436 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43436/console) for   PR 8982 at commit [`f83d86c`](https://github.com/apache/spark/commit/f83d86ce01c3943595518503230e283b3062a44d).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-145599419
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146971752
  
      [Test build #43481 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43481/console) for   PR 8982 at commit [`d56897e`](https://github.com/apache/spark/commit/d56897e0d3d5f0a2a007eeb3dda92d7f5a00d29c).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41439584
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
    @@ -278,3 +279,32 @@ case object MsSqlServerDialect extends JdbcDialect {
         } else None
       }
     }
    +
    +/**
    + * :: DeveloperApi ::
    + * Default Apache Derby dialect, mapping real on read
    + * and string/byte/short/boolean/decimal on write.
    + */
    +@DeveloperApi
    +case object DerbyDialect extends JdbcDialect {
    +  override def canHandle(url: String): Boolean = url.startsWith("jdbc:derby")
    +  override def getCatalystType(
    +    sqlType: Int, typeName: String, size: Int, md: MetadataBuilder): Option[DataType] = {
    +    if (sqlType == Types.REAL) {
    --- End diff --
    
    either
    ```scala
    if (sqlType == Types.REAL) Option(FloatType) else None
    ```
    or
    ```scala
    if (sqlType == Types.REAL) {
      Option(FloatType)
    } else {
      None
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-146926931
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/8982#issuecomment-145674671
  
    Merged build started.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-10855] [SQL] Add a JDBC dialect for Apa...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/8982#discussion_r41439621
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
    @@ -452,11 +456,19 @@ class JDBCSuite extends SparkFunSuite with BeforeAndAfter with SharedSQLContext
         assert(db2Dialect.getJDBCType(BooleanType).map(_.databaseTypeDefinition).get == "CHAR(1)")
       }
     
    +  test("DerbyDialect jdbc type mapping") {
    +    val derbyDialect = JdbcDialects.get("jdbc:derby:db")
    +    assert(derbyDialect.getJDBCType(StringType).map(_.databaseTypeDefinition).get == "CLOB")
    --- End diff --
    
    use ===, not ==


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org