You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by actuaryzhang <gi...@git.apache.org> on 2017/02/28 20:39:04 UTC

[GitHub] spark pull request #17105: [SPARK-19773][SparkR] SparkDataFrame should not a...

GitHub user actuaryzhang opened a pull request:

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

    [SPARK-19773][SparkR] SparkDataFrame should not allow duplicate names

    ## What changes were proposed in this pull request?
    
    SparkDataFrame in SparkR seems to accept duplicate names at creation, but incurs error when calling methods downstream. For example, we can do: 
    ```
    l <- list(list(1, 2), list(3, 4))
    df <- createDataFrame(l, c("a", "a"))
    head(df)
    ```
    But an error occurs when we do `df$a = df$a * 2.0`.
    
    In this PR, I add validity check for duplicate names at initialization. 
    
    ## How was this patch tested?
    new tests.

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

    $ git pull https://github.com/actuaryzhang/spark sparkRDataFrameValid

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

    https://github.com/apache/spark/pull/17105.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 #17105
    
----
commit f0f2dbb0cdcb1ba5f43a517b802b1a75f4f6563c
Author: actuaryzhang <ac...@gmail.com>
Date:   2017-02-28T19:25:55Z

    add dataframe validity check

commit 21b8b4d87b67832b51fffaa0b84b3a05f5f66dcc
Author: actuaryzhang <ac...@gmail.com>
Date:   2017-02-28T20:36:05Z

    add tests

----


---
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 #17105: [SPARK-19773][SparkR] SparkDataFrame should not allow du...

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

    https://github.com/apache/spark/pull/17105
  
    **[Test build #73609 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/73609/testReport)** for PR 17105 at commit [`df61c70`](https://github.com/apache/spark/commit/df61c70dcb2c46e7dcbe3caf2d708692075d05b3).


---
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 #17105: [SPARK-19773][SparkR] SparkDataFrame should not allow du...

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

    https://github.com/apache/spark/pull/17105
  
    @felixcheung 


---
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 #17105: [SPARK-19773][SparkR] SparkDataFrame should not a...

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

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


---
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 #17105: [SPARK-19773][SparkR] SparkDataFrame should not allow du...

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

    https://github.com/apache/spark/pull/17105
  
    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 #17105: [SPARK-19773][SparkR] SparkDataFrame should not allow du...

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

    https://github.com/apache/spark/pull/17105
  
    @felixcheung Ahh, it seems that we have some conflicting design issues. 
    
    1. From the test in collect() and crossJoin, it seems to allow dup names in SparkDataFrame by design:
    ```
      # collect() correctly handles multiple columns with same name
      df <- createDataFrame(list(list(1, 2)), schema = c("name", "name"))
      ldf <- collect(df)
      expect_equal(names(ldf), c("name", "name"))
    ```
    
    2. However, it seems that the `mutate` method does not allow dup names:
    ```
                # Check if there is any duplicated column name in the DataFrame
                dfCols <- columns(x)
                if (length(unique(dfCols)) != length(dfCols)) {
                  stop("Error: found duplicated column name in the DataFrame")
                }
    ```
    
    Wonder if you know the reasoning for such conflicting design? I think it's best to not allow dup names as it does not work with Spark SQL (which does not allow dup names). For example, we can not even extract the columns which will report error:
    ```
    l <- list(list(1, 2), list(3, 4))
    df <- createDataFrame(l, c("a", "a"))
    df$a
    ```
    What do you think? 



---
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 #17105: [SPARK-19773][SparkR] SparkDataFrame should not allow du...

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

    https://github.com/apache/spark/pull/17105
  
    so tl; dr; I think we should support duplicated name like everything else in Spark does.


---
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 #17105: [SPARK-19773][SparkR] SparkDataFrame should not allow du...

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

    https://github.com/apache/spark/pull/17105
  
    @actuaryzhang there's a bit of a history about this... but long story short, Spark does support DataFrame with multiple columns having the same name, for example
    ```
    # in pyspark
    >>> from pyspark.sql import Row
    >>> from pyspark.sql.types import *
    >>> data = [(1, 2, 'Foo')]
    >>> df = spark.createDataFrame(data, ("key", "key", "value"))
    >>> df
    DataFrame[key: bigint, key: bigint, value: string]
    ```
    
    And each column will get a unique id, so underneath the cover they are not actually "duplicating".
    
    You could in fact get columns with same name when doing a self-join, for instance.
    
    Now the reason why you are getting an error with `df$a = df$a * 2.0` is because "a" is not a full unique id. You get the same in python
    
    ```
    >>> df.select(col("key"))
    ...
        raise AnalysisException(s.split(': ', 1)[1], stackTrace)
    pyspark.sql.utils.AnalysisException: u"Reference 'key' is ambiguous, could be: key#0L, key#1L.;"
    ```
    
    And in R, `df$a` is essentially a shortcut to that and so it will also fail similarly.
    
    As for why it is disallowed in `mutate` - it is just a factor of the implementation. I think we could potentially implement it to support duplicated names.



---
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 #17105: [SPARK-19773][SparkR] SparkDataFrame should not allow du...

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

    https://github.com/apache/spark/pull/17105
  
    @felixcheung Thanks for the clarification. I will close this then. 


---
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 #17105: [SPARK-19773][SparkR] SparkDataFrame should not allow du...

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

    https://github.com/apache/spark/pull/17105
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/73609/
    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 #17105: [SPARK-19773][SparkR] SparkDataFrame should not allow du...

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

    https://github.com/apache/spark/pull/17105
  
    **[Test build #73609 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/73609/testReport)** for PR 17105 at commit [`df61c70`](https://github.com/apache/spark/commit/df61c70dcb2c46e7dcbe3caf2d708692075d05b3).
     * This patch **fails SparkR 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