You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by chenghao-intel <gi...@git.apache.org> on 2014/12/16 02:30:59 UTC

[GitHub] spark pull request: [SPARK-4856] [SQL] NullType instead of StringT...

GitHub user chenghao-intel opened a pull request:

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

    [SPARK-4856] [SQL] NullType instead of StringType when sampling against empty string or nul...

    ```
    TestSQLContext.sparkContext.parallelize(
      """{"ip":"27.31.100.29","headers":{"Host":"1.abc.com","Charset":"UTF-8"}}""" ::
      """{"ip":"27.31.100.29","headers":{}}""" ::
      """{"ip":"27.31.100.29","headers":""}""" :: Nil)
    ```
    As empty string (the "headers") will be considered as String in the beginning (in line 2 and 3), it ignores the real nested data type (struct type "headers" in line 1), and also take the line 1 (the "headers") as String Type, which is not our expected.

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

    $ git pull https://github.com/chenghao-intel/spark json

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

    https://github.com/apache/spark/pull/3708.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 #3708
    
----
commit 853de511ab1f695477c15d38324a6e34fb70ef35
Author: Cheng Hao <ha...@intel.com>
Date:   2014-12-16T01:12:22Z

    NullType instead of StringType when sampling against empty string or null value

----


---
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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#discussion_r21935456
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/json/JsonRDD.scala ---
    @@ -263,6 +263,8 @@ private[sql] object JsonRDD extends Logging {
             val elementType = typeOfArray(array)
             buildKeyPathForInnerStructs(array, elementType) :+ (key, elementType)
           }
    +      // we couldn't tell what the type is if the value is null or empty string
    +      case (key: String, value) if value == "" || value == null => (key, NullType) :: Nil
    --- End diff --
    
    The null case makes sense to me, but why `""` as well?  That seems to be unequivocally a `String`


---
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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#issuecomment-67266551
  
      [Test build #24519 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24519/consoleFull) for   PR 3708 at commit [`e7a72e9`](https://github.com/apache/spark/commit/e7a72e9006dceef0cd823391e9e6ab3d4ffda572).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class Analyzer(catalog: Catalog, registry: FunctionRegistry, caseSensitive: Boolean)`



---
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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#issuecomment-67111050
  
      [Test build #24481 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24481/consoleFull) for   PR 3708 at commit [`853de51`](https://github.com/apache/spark/commit/853de511ab1f695477c15d38324a6e34fb70ef35).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class Analyzer(catalog: Catalog, registry: FunctionRegistry, caseSensitive: Boolean)`



---
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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#issuecomment-67413213
  
    Thanks!  Merged to master.


---
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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#issuecomment-67111052
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24481/
    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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#issuecomment-67106388
  
    retest this please


---
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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#issuecomment-67266557
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24519/
    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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#discussion_r21945992
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/json/JsonRDD.scala ---
    @@ -263,6 +263,8 @@ private[sql] object JsonRDD extends Logging {
             val elementType = typeOfArray(array)
             buildKeyPathForInnerStructs(array, elementType) :+ (key, elementType)
           }
    +      // we couldn't tell what the type is if the value is null or empty string
    +      case (key: String, value) if value == "" || value == null => (key, NullType) :: Nil
    --- End diff --
    
    I've updated the unit test, which probably make more sense.
    
    In some cases (as shown in the unit test), `""` is equivalent to `null` for struct type, so we'd better not to say "it's MUST be `StringType` if we meet an empty string".
    
    In the meantime, the `NullType` is the `minimum` data type, and it can be promoted to any other data type in `JsonRDD` (e.g. promote to StructType), however, it's impossible to promote a `StringType` to `StructType`.
    
    It's safe to make it as `NullType` here, as we can promote it as `StringType` in the last promote rules, see https://github.com/chenghao-intel/spark/blob/json/sql/core/src/main/scala/org/apache/spark/sql/SQLContext.scala#L231


---
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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#issuecomment-67260799
  
      [Test build #24519 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24519/consoleFull) for   PR 3708 at commit [`e7a72e9`](https://github.com/apache/spark/commit/e7a72e9006dceef0cd823391e9e6ab3d4ffda572).
     * This patch merges cleanly.


---
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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#issuecomment-67106729
  
      [Test build #24481 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24481/consoleFull) for   PR 3708 at commit [`853de51`](https://github.com/apache/spark/commit/853de511ab1f695477c15d38324a6e34fb70ef35).
     * This patch merges cleanly.


---
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-4856] [SQL] NullType instead of StringT...

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

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


---
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-4856] [SQL] NullType instead of StringT...

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

    https://github.com/apache/spark/pull/3708#issuecomment-67098763
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24479/
    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