You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@spark.apache.org by HyukjinKwon <gi...@git.apache.org> on 2016/07/10 08:17:43 UTC

[GitHub] spark pull request #14124: [SPARK-16472][SQL] Inconsistent nullability in sc...

GitHub user HyukjinKwon opened a pull request:

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

    [SPARK-16472][SQL] Inconsistent nullability in schema after being read in SQL API

    ## What changes were proposed in this pull request?
    
    It seems the data sources implementing `FileFormat` seems loading the data by forcing the fields as nullable fields. (See here, https://github.com/apache/spark/blob/7374e518e2641fddfe57003340db410224b37581/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSource.scala#L402-L409)
    
    It seems this was official documented SPARK-11360 and was discussed here https://www.mail-archive.com/user@spark.apache.org/msg39230.html
    
    However, I realised that several APIs do not follow this. For example,
    
    ```scala
    DataFrame.json(jsonRDD: RDD[String])
    ```
    
    So, the codes below:
    
    ```scala
    val rdd = spark.sparkContext.makeRDD(Seq("{\"a\" : 1}", "{\"a\" : null}"))
    val schema = StructType(StructField("a", IntegerType, nullable = false) :: Nil)
    val df = spark.read.schema(schema).json(rdd)
    df.printSchema()
    ```
    
    prints below:
    
    ```
    root
     |-- a: integer (nullable = false)
    ```
    
    This API loads the schema as it is after loading. However, the schema became different when loading it by the API below (nullable fields) :
    
    ```
    spark.read.format("json").schema(...).load(path).printSchema()
    ```
    
    ```
    spark.read.schema(...).load(path).printSchema()
    ```
    
    produce below:
    
    ```
    root
     |-- a: integer (nullable = true)
    ```
    
    In addition, this is happening for structured streaming as well. (even when we read batch after writing it by structured streaming).
    
    I wrote some more such cases in the tests. (Structured Streaming). Please refer the tests codes below.
    
    ## How was this patch tested?
    
    Unit tests in `JsonSuite`, `FileStreamSinkSuite`, `FileStreamSourceSuite`, `HadoopFsRelationTest`.
    


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

    $ git pull https://github.com/HyukjinKwon/spark SPARK-16472

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

    https://github.com/apache/spark/pull/14124.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 #14124
    
----
commit a917678886779f236b1feffa23a11529ce67e97c
Author: hyukjinkwon <gu...@gmail.com>
Date:   2016-07-10T08:12:27Z

    Inconsistent nullability in schema after being read in SQL API

----


---
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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by HyukjinKwon <gi...@git.apache.org>.

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    @viirya Thanks for your comment! Actually, that's I want to have some feedback for from @marmbrus .
    
    It seems forcing to a nullable schema all is already happening when you read/write data via `read`/`write` API (but not for structured streaming and another API for json).
    
    So, actually, the reason of this PR is, to make all consistent. The reason to make them consistent in a way that the schema is forced as nullable is what he said in the mailing list.
    
    >Sure, but a traditional RDBMS has the opportunity to do validation before
    >loading data in.  Thats not really an option when you are reading random
    >files from S3.  This is why Hive and many other systems in this space treat
    >all columns as nullable.
    
    Actually, Parquet also reads and writes the schema with nullability correctly if we get rid of `asNullable` (I tested this before) but it seems that's prevented due to (I assume) the reason above.
    
    @marmbrus Do you mind if I ask to clarify here please?
    
    I think we may have to deal with this as datasource-specific problem.



---
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 #14124: [SPARK-16472][SQL] Force user specified schema to the nu...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by viirya <gi...@git.apache.org>.

Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    @HyukjinKwon Your patch solves this inconsistency by forcing schema as nullable at all. However,  looks like the parquet case is for compatibility, is this the same for json?


---
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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    **[Test build #64066 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/64066/consoleFull)** for PR 14124 at commit [`079aae2`](https://github.com/apache/spark/commit/079aae2e1f5a94ed4cb06ab797fa27951205a328).
     * 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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by HyukjinKwon <gi...@git.apache.org>.

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    I am a bit confused if we are allowed to read JSON (via `json(jsonRDD: RDD[String])` API) with schema having fields set `false` in `nullable`.
    If it is meant to be not allowed, this issue will prevents the case above. 
    
    But, yea, I think I agree that it is a potential problem anyway (even if the case above is not allowed.)


---
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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    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 #14124: [SPARK-16472][SQL] Force user specified schema to the nu...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    **[Test build #70922 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70922/testReport)** for PR 14124 at commit [`1abaf1b`](https://github.com/apache/spark/commit/1abaf1baeb8e45586b4fbcd8bc2786259e1170ab).
     * 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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by HyukjinKwon <gi...@git.apache.org>.

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    Actually, nvm. I think handling this in `DataFrameReader.schema` will deal with most of general cases.


---
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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by HyukjinKwon <gi...@git.apache.org>.

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    BTW, actually, this is not only about user-given schema.
    
    Currently, it always reads data into dataframe by datasources based on `FileFormat` ignoring nullability in schema (for both user-given schema and inferred/read schema).
    
    However, this does not happen when reading for streaming by the datasources (and another JSON api).
    
    So, this PR tries to make them consistent to ignore the nullability in schema.


---
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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by gatorsmile <gi...@git.apache.org>.

Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    ```
    val rdd = spark.sparkContext.makeRDD(Seq("{\"a\" : 1}", "{\"a\" : null}"))
    val schema = StructType(StructField("a", IntegerType, nullable = false) :: Nil)
    val df = spark.read.schema(schema).json(rdd)
    df.printSchema()
    ```
    
    When user-specified schemas are not nullable and the data contains null, the null value in the result becomes `0`. This looks like a bug, right?


---
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 #14124: [SPARK-16472][SQL] Force user specified schema to the nu...

Posted by HyukjinKwon <gi...@git.apache.org>.

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    Let me rather close this for a while. I will reopen if it looks worth. I think the same subject could be discussed in @kiszk 's PR.


---
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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by HyukjinKwon <gi...@git.apache.org>.

Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    Thanks for feedback @cloud-fan !
    
    If the user-given schema is wrong, it is handled differently for each datasource specific.
    
    - For JSON and CSV
      it is kind of permissive generally (for example, compatibility among numeric types).
    
    - For ORC and Parquet
      Generally it is strict to types. So they don't allow the compatibility (except for very few cases, e.g. for parquet, https://github.com/apache/spark/pull/14272 and https://github.com/apache/spark/pull/14278)
    
    I think so. Should we disallow specifying schemas for these? 
    - For JDBC
      it does not take user-given schema since it does not implement `SchemaRelationProvider`.
    



---
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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    **[Test build #62053 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62053/consoleFull)** for PR 14124 at commit [`adae8de`](https://github.com/apache/spark/commit/adae8de39ffcec8ca3785c1123da900a457691c1).
     * This patch **fails PySpark unit 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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    Merged build finished. Test FAILed.


---
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 #14124: [SPARK-16472][SQL] Inconsistent nullability in schema af...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/14124
  
    **[Test build #62054 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62054/consoleFull)** for PR 14124 at commit [`3980681`](https://github.com/apache/spark/commit/39806815fbbef2aafb32d3173c23386fcfbc5edf).
     * 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