You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2016/06/27 04:46:03 UTC

[GitHub] spark pull request #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

GitHub user gatorsmile opened a pull request:

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

    [SPARK-16222] [SQL] JDBC Sources - Handling illegal input values for `fetchsize` and `batchsize`

    #### What changes were proposed in this pull request?
    For JDBC data sources, users can specify `batchsize` for multi-row inserts and `fetchsize` for multi-row fetch. A few issues exist:
    
    - The property keys are case sensitive. Thus, the existing test cases for `fetchsize` use incorrect names. Basically, the test cases are broken. 
    - No test cases exist for `batchsize`. 
    - We do not detect the illegal input values for `fetchsize` and `batchsize`. 
    
    For example, when `batchsize` is zero, we got the following exception: 
    ```
    Job aborted due to stage failure: Task 0 in stage 0.0 failed 1 times, most recent failure: Lost task 0.0 in stage 0.0 (TID 0, localhost): java.lang.ArithmeticException: / by zero
    ```
    when `fetchsize` is less than zero, we got the exception from the underlying JDBC driver:
    ```
    Job aborted due to stage failure: Task 0 in stage 0.0 failed 1 times, most recent failure: Lost task 0.0 in stage 0.0 (TID 0, localhost): org.h2.jdbc.JdbcSQLException: Invalid value "-1" for parameter "rows" [90008-183]
    ```
    
    This PR fixes all the above issues, and issue the appropriate exceptions when detecting the illegal inputs for `fetchsize` and `batchsize`. Also update the function descriptions.
    
    #### How was this patch tested?
    Test cases are fixed and added.

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

    $ git pull https://github.com/gatorsmile/spark jdbcProperties

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

    https://github.com/apache/spark/pull/13919.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 #13919
    
----
commit 852c0e9bd7ecebf1bce1906ad641d8f714b9fe45
Author: gatorsmile <ga...@gmail.com>
Date:   2016-06-27T01:44:25Z

    batchsize and fetchsize

commit cf2ce283407c62f3fd4d64cf380ddfcf262d7ecc
Author: gatorsmile <ga...@gmail.com>
Date:   2016-06-27T02:18:36Z

    fix

----


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69079897
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
    @@ -351,11 +352,23 @@ class JDBCSuite extends SparkFunSuite
           urlWithUserAndPass, "TEST.PEOPLE", new Properties).collect().length === 3)
       }
     
    -  test("Basic API with FetchSize") {
    +  test("Basic API with illegal FetchSize") {
         val properties = new Properties
    -    properties.setProperty("fetchSize", "2")
    -    assert(spark.read.jdbc(
    -      urlWithUserAndPass, "TEST.PEOPLE", properties).collect().length === 3)
    +    properties.setProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "-1")
    +    val e = intercept[SparkException] {
    +      spark.read.jdbc(urlWithUserAndPass, "TEST.PEOPLE", properties).collect()
    +    }.getMessage
    +    assert(e.contains("Invalid value `-1` for parameter `fetchsize`. The minimum value is zero. " +
    +      "When the value is zero, the JDBC driver ignores the value and does the estimates."))
    +  }
    +
    +  test("Basic API with FetchSize") {
    +    (0 to 4).foreach { size =>
    +      val properties = new Properties
    --- End diff --
    
    Nit: constructors have side effects so `new Properties()` is idiomatic


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69030522
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
    @@ -390,7 +390,9 @@ private[sql] class JDBCRDD(
         val sqlText = s"SELECT $columnList FROM $fqTable $myWhereClause"
         val stmt = conn.prepareStatement(sqlText,
             ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)
    -    val fetchSize = properties.getProperty("fetchsize", "0").toInt
    +    val fetchSize = properties.getProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "0").toInt
    +    require(fetchSize >= 0,
    +      s"Invalid value `${fetchSize.toString}` for parameter `${JdbcUtils.JDBC_BATCH_FETCH_SIZE}`.")
    --- End diff --
    
    sorry what i meant was to say the valid range in the error message


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69079853
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
    @@ -351,11 +352,23 @@ class JDBCSuite extends SparkFunSuite
           urlWithUserAndPass, "TEST.PEOPLE", new Properties).collect().length === 3)
       }
     
    -  test("Basic API with FetchSize") {
    +  test("Basic API with illegal FetchSize") {
         val properties = new Properties
    -    properties.setProperty("fetchSize", "2")
    -    assert(spark.read.jdbc(
    -      urlWithUserAndPass, "TEST.PEOPLE", properties).collect().length === 3)
    +    properties.setProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "-1")
    +    val e = intercept[SparkException] {
    +      spark.read.jdbc(urlWithUserAndPass, "TEST.PEOPLE", properties).collect()
    +    }.getMessage
    +    assert(e.contains("Invalid value `-1` for parameter `fetchsize`. The minimum value is zero. " +
    --- End diff --
    
    I think this assertion is too brittle because it asserts the entire exact message. I can see asserting that it contains "fetchsize" and "-1" or soemthing


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69080040
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
    @@ -390,7 +390,11 @@ private[sql] class JDBCRDD(
         val sqlText = s"SELECT $columnList FROM $fqTable $myWhereClause"
         val stmt = conn.prepareStatement(sqlText,
             ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)
    -    val fetchSize = properties.getProperty("fetchsize", "0").toInt
    +    val fetchSize = properties.getProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "0").toInt
    +    require(fetchSize >= 0,
    +      s"Invalid value `${fetchSize.toString}` for parameter " +
    +      s"`${JdbcUtils.JDBC_BATCH_FETCH_SIZE}`. The minimum value is zero. When the value is zero, " +
    --- End diff --
    
    Nit: zero => 0. I think we use the words, in general, when talking about the idea rather than the integer. Thin distinction but I'd say "we have two options" but "input must be at least 2"


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69029680
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
    @@ -390,7 +390,9 @@ private[sql] class JDBCRDD(
         val sqlText = s"SELECT $columnList FROM $fqTable $myWhereClause"
         val stmt = conn.prepareStatement(sqlText,
             ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)
    -    val fetchSize = properties.getProperty("fetchsize", "0").toInt
    +    val fetchSize = properties.getProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "0").toInt
    +    require(fetchSize >= 0,
    +      s"Invalid value `${fetchSize.toString}` for parameter `${JdbcUtils.JDBC_BATCH_FETCH_SIZE}`.")
    --- End diff --
    
    https://github.com/gatorsmile/spark/blob/b999b8a1474fc9d4f8d3a4c94694fbc40572111a/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L181-L183 
    
    https://github.com/gatorsmile/spark/blob/b999b8a1474fc9d4f8d3a4c94694fbc40572111a/sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala#L394-L395
    
    Above are the document changes. Please check if they are ok. 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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r68672678
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCWriteSuite.scala ---
    @@ -96,6 +98,30 @@ class JDBCWriteSuite extends SharedSQLContext with BeforeAndAfter {
           2 === spark.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).collect()(0).length)
       }
     
    +  test("Basic CREATE with illegal batchsize") {
    +    val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
    +
    +    (-1 to 0).foreach { size =>
    +      val properties = new Properties
    +      properties.setProperty(JdbcUtils.JDBC_BATCH_INSERT_SIZE, size.toString)
    +      val e = intercept[SparkException] {
    +        df.write.mode(SaveMode.Overwrite).jdbc(url, "TEST.BASICCREATETEST", properties)
    +      }.getMessage
    +      assert(e.contains(s"Invalid value `$size` for parameter `batchsize`"))
    +    }
    +  }
    +
    +  test("Basic CREATE with batchsize") {
    +    val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
    +
    +    (1 to 3).foreach { size =>
    +      val properties = new Properties
    +      properties.setProperty(JdbcUtils.JDBC_BATCH_INSERT_SIZE, size.toString)
    +      df.write.mode(SaveMode.Overwrite).jdbc(url, "TEST.BASICCREATETEST", properties)
    +      assert(2 === spark.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).count)
    --- End diff --
    
    add count()


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r68672636
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
    @@ -390,7 +390,9 @@ private[sql] class JDBCRDD(
         val sqlText = s"SELECT $columnList FROM $fqTable $myWhereClause"
         val stmt = conn.prepareStatement(sqlText,
             ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)
    -    val fetchSize = properties.getProperty("fetchsize", "0").toInt
    +    val fetchSize = properties.getProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "0").toInt
    +    require(fetchSize >= 0,
    +      s"Invalid value `${fetchSize.toString}` for parameter `${JdbcUtils.JDBC_BATCH_FETCH_SIZE}`.")
    --- End diff --
    
    document what the valid ranges are


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/13919
  
    cc @srowen @rxin 


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

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


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61286/
    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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69030810
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCWriteSuite.scala ---
    @@ -96,6 +98,30 @@ class JDBCWriteSuite extends SharedSQLContext with BeforeAndAfter {
           2 === spark.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).collect()(0).length)
       }
     
    +  test("Basic CREATE with illegal batchsize") {
    +    val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
    +
    +    (-1 to 0).foreach { size =>
    +      val properties = new Properties
    +      properties.setProperty(JdbcUtils.JDBC_BATCH_INSERT_SIZE, size.toString)
    +      val e = intercept[SparkException] {
    +        df.write.mode(SaveMode.Overwrite).jdbc(url, "TEST.BASICCREATETEST", properties)
    +      }.getMessage
    +      assert(e.contains(s"Invalid value `$size` for parameter `batchsize`"))
    +    }
    +  }
    +
    +  test("Basic CREATE with batchsize") {
    +    val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
    +
    +    (1 to 3).foreach { size =>
    +      val properties = new Properties
    +      properties.setProperty(JdbcUtils.JDBC_BATCH_INSERT_SIZE, size.toString)
    +      df.write.mode(SaveMode.Overwrite).jdbc(url, "TEST.BASICCREATETEST", properties)
    +      assert(2 === spark.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).count)
    --- End diff --
    
    Uh... I see. Let me do it. 


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69081338
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala ---
    @@ -154,6 +158,10 @@ object JdbcUtils extends Logging {
           nullTypes: Array[Int],
           batchSize: Int,
           dialect: JdbcDialect): Iterator[Byte] = {
    +    require(batchSize >= 1,
    +      s"Invalid value `${batchSize.toString}` for parameter " +
    +      s"`${JdbcUtils.JDBC_BATCH_INSERT_SIZE}`. The minimum value is one.")
    --- End diff --
    
    Great!


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/13919
  
    @rxin Please feel free to let me know if any change is needed. 


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69030861
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
    @@ -390,7 +390,9 @@ private[sql] class JDBCRDD(
         val sqlText = s"SELECT $columnList FROM $fqTable $myWhereClause"
         val stmt = conn.prepareStatement(sqlText,
             ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)
    -    val fetchSize = properties.getProperty("fetchsize", "0").toInt
    +    val fetchSize = properties.getProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "0").toInt
    +    require(fetchSize >= 0,
    +      s"Invalid value `${fetchSize.toString}` for parameter `${JdbcUtils.JDBC_BATCH_FETCH_SIZE}`.")
    --- End diff --
    
    Sure, let me add it. 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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/13919
  
    Seems reasonable


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61448/
    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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69081362
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
    @@ -390,7 +390,11 @@ private[sql] class JDBCRDD(
         val sqlText = s"SELECT $columnList FROM $fqTable $myWhereClause"
         val stmt = conn.prepareStatement(sqlText,
             ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)
    -    val fetchSize = properties.getProperty("fetchsize", "0").toInt
    +    val fetchSize = properties.getProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "0").toInt
    +    require(fetchSize >= 0,
    +      s"Invalid value `${fetchSize.toString}` for parameter " +
    +      s"`${JdbcUtils.JDBC_BATCH_FETCH_SIZE}`. The minimum value is zero. When the value is zero, " +
    --- End diff --
    
    Yeah, will follow it. 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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    **[Test build #61448 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61448/consoleFull)** for PR 13919 at commit [`b999b8a`](https://github.com/apache/spark/commit/b999b8a1474fc9d4f8d3a4c94694fbc40572111a).


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r68673001
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCRDD.scala ---
    @@ -390,7 +390,9 @@ private[sql] class JDBCRDD(
         val sqlText = s"SELECT $columnList FROM $fqTable $myWhereClause"
         val stmt = conn.prepareStatement(sqlText,
             ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY)
    -    val fetchSize = properties.getProperty("fetchsize", "0").toInt
    +    val fetchSize = properties.getProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "0").toInt
    +    require(fetchSize >= 0,
    +      s"Invalid value `${fetchSize.toString}` for parameter `${JdbcUtils.JDBC_BATCH_FETCH_SIZE}`.")
    --- End diff --
    
    Sure, will do it. 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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69081296
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
    @@ -351,11 +352,23 @@ class JDBCSuite extends SparkFunSuite
           urlWithUserAndPass, "TEST.PEOPLE", new Properties).collect().length === 3)
       }
     
    -  test("Basic API with FetchSize") {
    +  test("Basic API with illegal FetchSize") {
         val properties = new Properties
    -    properties.setProperty("fetchSize", "2")
    -    assert(spark.read.jdbc(
    -      urlWithUserAndPass, "TEST.PEOPLE", properties).collect().length === 3)
    +    properties.setProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "-1")
    +    val e = intercept[SparkException] {
    +      spark.read.jdbc(urlWithUserAndPass, "TEST.PEOPLE", properties).collect()
    +    }.getMessage
    +    assert(e.contains("Invalid value `-1` for parameter `fetchsize`. The minimum value is zero. " +
    --- End diff --
    
    Sure, will do it. 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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r68673016
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCWriteSuite.scala ---
    @@ -96,6 +98,30 @@ class JDBCWriteSuite extends SharedSQLContext with BeforeAndAfter {
           2 === spark.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).collect()(0).length)
       }
     
    +  test("Basic CREATE with illegal batchsize") {
    +    val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
    +
    +    (-1 to 0).foreach { size =>
    +      val properties = new Properties
    +      properties.setProperty(JdbcUtils.JDBC_BATCH_INSERT_SIZE, size.toString)
    +      val e = intercept[SparkException] {
    +        df.write.mode(SaveMode.Overwrite).jdbc(url, "TEST.BASICCREATETEST", properties)
    +      }.getMessage
    +      assert(e.contains(s"Invalid value `$size` for parameter `batchsize`"))
    +    }
    +  }
    +
    +  test("Basic CREATE with batchsize") {
    +    val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
    +
    +    (1 to 3).foreach { size =>
    +      val properties = new Properties
    +      properties.setProperty(JdbcUtils.JDBC_BATCH_INSERT_SIZE, size.toString)
    +      df.write.mode(SaveMode.Overwrite).jdbc(url, "TEST.BASICCREATETEST", properties)
    +      assert(2 === spark.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).count)
    --- End diff --
    
    Will do. 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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69030431
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCWriteSuite.scala ---
    @@ -96,6 +98,30 @@ class JDBCWriteSuite extends SharedSQLContext with BeforeAndAfter {
           2 === spark.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).collect()(0).length)
       }
     
    +  test("Basic CREATE with illegal batchsize") {
    +    val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
    +
    +    (-1 to 0).foreach { size =>
    +      val properties = new Properties
    +      properties.setProperty(JdbcUtils.JDBC_BATCH_INSERT_SIZE, size.toString)
    +      val e = intercept[SparkException] {
    +        df.write.mode(SaveMode.Overwrite).jdbc(url, "TEST.BASICCREATETEST", properties)
    +      }.getMessage
    +      assert(e.contains(s"Invalid value `$size` for parameter `batchsize`"))
    +    }
    +  }
    +
    +  test("Basic CREATE with batchsize") {
    +    val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
    +
    +    (1 to 3).foreach { size =>
    +      val properties = new Properties
    +      properties.setProperty(JdbcUtils.JDBC_BATCH_INSERT_SIZE, size.toString)
    +      df.write.mode(SaveMode.Overwrite).jdbc(url, "TEST.BASICCREATETEST", properties)
    +      assert(2 === spark.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).count)
    --- End diff --
    
    what I meant was -- add parentheses to count


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    **[Test build #61550 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61550/consoleFull)** for PR 13919 at commit [`308d51c`](https://github.com/apache/spark/commit/308d51c09ba6c20af4b0a7333a547b7c3f616404).
     * 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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    **[Test build #61503 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61503/consoleFull)** for PR 13919 at commit [`00c744c`](https://github.com/apache/spark/commit/00c744cb3a2ac4f246ffa8425f33be660a8fe715).


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    **[Test build #61448 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61448/consoleFull)** for PR 13919 at commit [`b999b8a`](https://github.com/apache/spark/commit/b999b8a1474fc9d4f8d3a4c94694fbc40572111a).
     * 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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69029404
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCWriteSuite.scala ---
    @@ -96,6 +98,30 @@ class JDBCWriteSuite extends SharedSQLContext with BeforeAndAfter {
           2 === spark.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).collect()(0).length)
       }
     
    +  test("Basic CREATE with illegal batchsize") {
    +    val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
    +
    +    (-1 to 0).foreach { size =>
    +      val properties = new Properties
    +      properties.setProperty(JdbcUtils.JDBC_BATCH_INSERT_SIZE, size.toString)
    +      val e = intercept[SparkException] {
    +        df.write.mode(SaveMode.Overwrite).jdbc(url, "TEST.BASICCREATETEST", properties)
    +      }.getMessage
    +      assert(e.contains(s"Invalid value `$size` for parameter `batchsize`"))
    +    }
    +  }
    +
    +  test("Basic CREATE with batchsize") {
    +    val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), schema2)
    +
    +    (1 to 3).foreach { size =>
    +      val properties = new Properties
    +      properties.setProperty(JdbcUtils.JDBC_BATCH_INSERT_SIZE, size.toString)
    +      df.write.mode(SaveMode.Overwrite).jdbc(url, "TEST.BASICCREATETEST", properties)
    +      assert(2 === spark.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).count)
    --- End diff --
    
    This line already has `count`. I guess what you want is `collect()`. The latest code change add it, as shown below: https://github.com/apache/spark/pull/13919/files#diff-4e7d97319461efb33f06b6d245254dcdR122 


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    **[Test build #61550 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61550/consoleFull)** for PR 13919 at commit [`308d51c`](https://github.com/apache/spark/commit/308d51c09ba6c20af4b0a7333a547b7c3f616404).


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/13919
  
    @srowen Thank you!


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    **[Test build #61286 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61286/consoleFull)** for PR 13919 at commit [`cf2ce28`](https://github.com/apache/spark/commit/cf2ce283407c62f3fd4d64cf380ddfcf262d7ecc).
     * 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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    **[Test build #61286 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61286/consoleFull)** for PR 13919 at commit [`cf2ce28`](https://github.com/apache/spark/commit/cf2ce283407c62f3fd4d64cf380ddfcf262d7ecc).


---
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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69079913
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala ---
    @@ -154,6 +158,10 @@ object JdbcUtils extends Logging {
           nullTypes: Array[Int],
           batchSize: Int,
           dialect: JdbcDialect): Iterator[Byte] = {
    +    require(batchSize >= 1,
    +      s"Invalid value `${batchSize.toString}` for parameter " +
    +      s"`${JdbcUtils.JDBC_BATCH_INSERT_SIZE}`. The minimum value is one.")
    --- End diff --
    
    Nit: one => 1


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61503/
    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 #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illeg...

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

    https://github.com/apache/spark/pull/13919#discussion_r69081332
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala ---
    @@ -351,11 +352,23 @@ class JDBCSuite extends SparkFunSuite
           urlWithUserAndPass, "TEST.PEOPLE", new Properties).collect().length === 3)
       }
     
    -  test("Basic API with FetchSize") {
    +  test("Basic API with illegal FetchSize") {
         val properties = new Properties
    -    properties.setProperty("fetchSize", "2")
    -    assert(spark.read.jdbc(
    -      urlWithUserAndPass, "TEST.PEOPLE", properties).collect().length === 3)
    +    properties.setProperty(JdbcUtils.JDBC_BATCH_FETCH_SIZE, "-1")
    +    val e = intercept[SparkException] {
    +      spark.read.jdbc(urlWithUserAndPass, "TEST.PEOPLE", properties).collect()
    +    }.getMessage
    +    assert(e.contains("Invalid value `-1` for parameter `fetchsize`. The minimum value is zero. " +
    +      "When the value is zero, the JDBC driver ignores the value and does the estimates."))
    +  }
    +
    +  test("Basic API with FetchSize") {
    +    (0 to 4).foreach { size =>
    +      val properties = new Properties
    --- End diff --
    
    I see. Will make a change.


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/13919
  
    Merged to master/2.0


---
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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/61550/
    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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

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

    https://github.com/apache/spark/pull/13919
  
    **[Test build #61503 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/61503/consoleFull)** for PR 13919 at commit [`00c744c`](https://github.com/apache/spark/commit/00c744cb3a2ac4f246ffa8425f33be660a8fe715).
     * 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 issue #13919: [SPARK-16222] [SQL] JDBC Sources - Handling illegal inpu...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/13919
  
    @srowen Could you please review the latest changes? 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