You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dilipbiswal <gi...@git.apache.org> on 2016/03/07 20:38:50 UTC

[GitHub] spark pull request: [SPARK-12719][SQL] SQL generation support for ...

GitHub user dilipbiswal opened a pull request:

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

    [SPARK-12719][SQL] SQL generation support for generators, including UDTF

    ## What changes were proposed in this pull request?
    This PR is to convert SQL from analyzed logical plans containing Generate operator.
    
    Sample Plan :
    ```
    GlobalLimit 3
    +- LocalLimit 3
       +- Project [gencol2#204]
          +- Generate explode(gencol1#203), true, false, Some(gentab2), [gencol2#204]
             +- Generate explode(array(array(1, 2, 3))), true, false, Some(gentab1), [gencol1#203]
                +- MetastoreRelation default, t4, None
    ```
    Generated Query:
    
    ```
    SELECT `gentab2`.`gencol2` FROM `default`.`t4` LATERAL VIEW explode(array(array(1, 2, 3))) `gentab1` AS `gencol1` LATERAL VIEW explode(`gentab1`.`gencol1`) `gentab2` AS `gencol2` LIMIT 3
    ```
    
    - Generators can be specified in either projection list or in the lateral view clause.
    - First case is handled in projectToSQL
    - The second case is handled in generateToSQL
    - Also contains the fix for spark-13698. As soon as it it merged , i will rebase and submit again.
    
    ## How was this patch tested?
    Added test cases in LogicalPlanToSQLSuite
    Have also run HiveCompatibilitySuite to look for any failure in generation of Generate plans.
    
    
    (If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
    


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

    $ git pull https://github.com/dilipbiswal/spark SPARK-12719

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

    https://github.com/apache/spark/pull/11563.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 #11563
    
----
commit d2aa88ae294fc76e256080b6f55a3b11b65f6de6
Author: Dilip Biswal <db...@us.ibm.com>
Date:   2016-03-05T09:49:58Z

    [SPARK-13698] Fix Analysis Exceptions when Using Backticks in Generate

commit d49d5667cebe2c1319159dd4470467349e8cf027
Author: Dilip Biswal <db...@us.ibm.com>
Date:   2016-03-07T07:42:00Z

    [SPARK-12719] SQL generation support for generators (including UDTF)

----


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#issuecomment-193416690
  
    cc @liancheng @gatorsmile 
    Thanks to @gatorsmile to let me work on this :-)


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

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


[GitHub] spark pull request: [SPARK-12719][SQL] SQL generation support for ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#issuecomment-193424438
  
    Overall, LGTM. Thanks!


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

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


[GitHub] spark pull request: [SPARK-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55296366
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/LogicalPlanToSQLSuite.scala ---
    @@ -445,4 +461,86 @@ class LogicalPlanToSQLSuite extends SQLBuilderTest with SQLTestUtils {
           "f1", "b[0].f1", "f1", "c[foo]", "d[0]"
         )
       }
    +
    +  test("SQL generation for generate") {
    --- End diff --
    
    @rxin ok Reynold. I will group them into different test 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 pull request: [SPARK-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#issuecomment-193670859
  
    Hi @dilipbiswal , could you describe the pattern of a logical plan containing `Generator`? And does the pattern differ when the generator is in projection list or in LATERAL VIEW?


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55275392
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/SQLBuilder.scala ---
    @@ -297,6 +322,31 @@ class SQLBuilder(logicalPlan: LogicalPlan, sqlContext: SQLContext) extends Loggi
         )
       }
     
    +  /* This function handles the SQL generation when generators are specified in the
    +   * LATERAL VIEW clause. SQL generation of generators specified in projection lists
    +   * are handled in projectToSQL.
    +   * sample plan :
    +   *   +- Project [mycol2#192]
    +   *     +- Generate explode(myCol#191), true, false, Some(mytable2), [mycol2#192]
    +   *       +- Generate explode(array(array(1, 2, 3))), true, false, Some(mytable), [mycol#191]
    +   *         +- MetastoreRelation default, src, None
    +   *
    +   */
    +  private def generateToSQL(plan: Generate): String = {
    +    val columnAliases = plan.generatorOutput.map(a => quoteIdentifier(a.name)).mkString(",")
    +    val generatorAlias = if (plan.qualifier.isEmpty) "" else plan.qualifier.get
    +    val outerClause = if (plan.outer) "OUTER" else ""
    +    build(
    +      toSQL(plan.child),
    +      "LATERAL VIEW ",
    --- End diff --
    
    Fixed.


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55260950
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/LogicalPlanToSQLSuite.scala ---
    @@ -45,6 +56,11 @@ class LogicalPlanToSQLSuite extends SQLBuilderTest with SQLTestUtils {
           .select('id as 'a, 'id as 'b, 'id as 'c, 'id as 'd)
           .write
           .saveAsTable("parquet_t2")
    +
    +    sqlContext.range(10).select('id as 'a, 'id as 'b, 'id as 'c, 'id as 'd).write.saveAsTable("t2")
    --- End diff --
    
    If they are parquet tables, you need to rename it. It might help others use 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: [SPARK-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55259898
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/CatalystQl.scala ---
    @@ -999,10 +999,16 @@ https://cwiki.apache.org/confluence/display/Hive/Enhanced+Aggregation%2C+Cube%2C
         }
     
         val attributes = clauses.collect {
    -      case Token(a, Nil) => UnresolvedAttribute(a.toLowerCase)
    +      case Token(a, Nil) => UnresolvedAttribute(cleanIdentifier(a.toLowerCase))
         }
     
    -    Generate(generator, join = true, outer = outer, Some(alias.toLowerCase), attributes, child)
    +    Generate(
    +      generator,
    --- End diff --
    
    4-space Indentations are needed here. 
    https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide 


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#issuecomment-193542558
  
    @rxin.  Hi Reynold,
    
    We have two cases to handle.
    ```SQL
    SELECT explode(array(1,2,3)) FROM src
    SELECT gentab2.* FROM t1 LATERAL VIEW explode(array(array(1,2,3))) gentab1 AS gencol1 LATERAL VIEW explode(gentab1.gencol1) gentab2 AS gencol2
    ```
    Currently, I handle the first case in `projToSql` and the 2nd case in `generateToSql`, 
    as we wanted to generate SQLs which is closer to the original SQL. 
    
    Lateral view also can refer to columns from tables before itself. So i felt it is safer to 
    generate the SQL very close to the source SQL to reduce any risk. I also thought 
    about treating the first case as a special case of LATERAL view. In this case we 
    had to handle the generation of a table alias which is missing in case-1 and fixing up
     the projection list above to refer to it.
    
    However, I went with the approach in this PR as it didn't seem too complex and also retained the layout of the original SQL. I could be easily overlooking something here and would appreciate your guidance. Please let me know.



---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#issuecomment-193505236
  
    Why do we need two separate cases here?


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

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


[GitHub] spark pull request: [SPARK-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55261621
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/CatalystQl.scala ---
    @@ -999,10 +999,16 @@ https://cwiki.apache.org/confluence/display/Hive/Enhanced+Aggregation%2C+Cube%2C
         }
     
         val attributes = clauses.collect {
    -      case Token(a, Nil) => UnresolvedAttribute(a.toLowerCase)
    +      case Token(a, Nil) => UnresolvedAttribute(cleanIdentifier(a.toLowerCase))
         }
     
    -    Generate(generator, join = true, outer = outer, Some(alias.toLowerCase), attributes, child)
    +    Generate(
    +      generator,
    --- End diff --
    
    @gatorsmile Thanks !! In the doc, 4-space indentation is applicable for function declaration. Is it also applicable here ?


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

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


[GitHub] spark pull request: [SPARK-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55275319
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/SQLBuilder.scala ---
    @@ -297,6 +322,31 @@ class SQLBuilder(logicalPlan: LogicalPlan, sqlContext: SQLContext) extends Loggi
         )
       }
     
    +  /* This function handles the SQL generation when generators are specified in the
    +   * LATERAL VIEW clause. SQL generation of generators specified in projection lists
    +   * are handled in projectToSQL.
    --- End diff --
    
    Done.


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#issuecomment-193674195
  
    I think making generated SQL closer to the original one is not our goal, instead the goal should be: make your approach as simple as possible, so that it's easy to reason about, verify the correctness, and review the code.


---
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-12719][SQL] SQL generation support for ...

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

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


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55260262
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/SQLBuilder.scala ---
    @@ -297,6 +322,31 @@ class SQLBuilder(logicalPlan: LogicalPlan, sqlContext: SQLContext) extends Loggi
         )
       }
     
    +  /* This function handles the SQL generation when generators are specified in the
    +   * LATERAL VIEW clause. SQL generation of generators specified in projection lists
    +   * are handled in projectToSQL.
    +   * sample plan :
    +   *   +- Project [mycol2#192]
    +   *     +- Generate explode(myCol#191), true, false, Some(mytable2), [mycol2#192]
    +   *       +- Generate explode(array(array(1, 2, 3))), true, false, Some(mytable), [mycol#191]
    +   *         +- MetastoreRelation default, src, None
    +   *
    +   */
    +  private def generateToSQL(plan: Generate): String = {
    +    val columnAliases = plan.generatorOutput.map(a => quoteIdentifier(a.name)).mkString(",")
    +    val generatorAlias = if (plan.qualifier.isEmpty) "" else plan.qualifier.get
    +    val outerClause = if (plan.outer) "OUTER" else ""
    +    build(
    +      toSQL(plan.child),
    +      "LATERAL VIEW ",
    --- End diff --
    
    ```
    "LATERAL VIEW "
    ```
    ->
    ```
    "LATERAL VIEW"
    ```


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55260725
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/LogicalPlanToSQLSuite.scala ---
    @@ -30,6 +30,17 @@ class LogicalPlanToSQLSuite extends SQLBuilderTest with SQLTestUtils {
         sql("DROP TABLE IF EXISTS parquet_t1")
         sql("DROP TABLE IF EXISTS parquet_t2")
         sql("DROP TABLE IF EXISTS t0")
    +    sql("DROP TABLE IF EXISTS t3")
    +    sql("DROP TABLE IF EXISTS t4")
    --- End diff --
    
    `t3`, `t4` -> `t2`, `t3`


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#issuecomment-193689584
  
    @cloud-fan Thanks..I understand now from Reynold's and your feedback. I wrongly assumed the goal to make the best effort to retain the original sql layout.
    
    Regarding the pattern of logical plans between the two cases here is
    what comes to my mind now. I will update if i remember more.
     
    * Projection List
        * The generate node has no qualifier. 
        * There can be one generator in the projection List
        * Join is not always true.
    * LATERAL VIEW
        * Generate has a qualifier (column names optional)
        * There can be multiple generators. So we need preserve the order of from clause.
        * Join is always true as there has to be a table before the LATERAL VIEW. This
            can cause ambiguity of column resolution if we express the projection list generators
           as lateral views and don't qualify the projection list properly.
           
    ```
    +- Project [gencol2#204]
          +- Generate explode(gencol1#203), true, false, Some(gentab2), [gencol2#204]
             +- Generate explode(array(array(1, 2, 3))), true, false, Some(gentab1), [gencol1#203]
                +- MetastoreRelation default, t4, None
    ```
    In the above plan, we have two LATERAL VIEW clauses with qualifier gentab1 and gentab2 respectively.
    
    ```
    Project [value#53]
    +- Generate explode(array(1, 2, 3)), false, false, None, [value#53]
       +- MetastoreRelation default, src, None
    ```
    
    In the above case, its a generator in the projection list. 
    
    There is a test which uses generator but has no table in the FROM clause like following.
    ```SQL
    select explode(array(1,2,3)) AS gencol
    ```
    I suspect this may cause problem if we need to express the generator as a LATERAL VIEW.
    
    Please let me know if you need any other info.
    
    



---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#issuecomment-196558236
  
    Closing this in favor of 
    [#11696] (http://github.com/apache/spark/pull/11696)


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#issuecomment-193572743
  
    "as we wanted to generate SQLs which is closer to the original SQL"
    
    Why is this a goal? I worry about the fragility of this two cases, if we really only need one to satisfy correctness.



---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55309116
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/LogicalPlanToSQLSuite.scala ---
    @@ -445,4 +461,86 @@ class LogicalPlanToSQLSuite extends SQLBuilderTest with SQLTestUtils {
           "f1", "b[0].f1", "f1", "c[foo]", "d[0]"
         )
       }
    +
    +  test("SQL generation for generate") {
    --- End diff --
    
    @rxin I have split the tests into 5 groups. Pl. let me know if it looks ok to 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 pull request: [SPARK-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55260972
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/LogicalPlanToSQLSuite.scala ---
    @@ -52,6 +68,8 @@ class LogicalPlanToSQLSuite extends SQLBuilderTest with SQLTestUtils {
         sql("DROP TABLE IF EXISTS parquet_t1")
         sql("DROP TABLE IF EXISTS parquet_t2")
         sql("DROP TABLE IF EXISTS t0")
    +    sql("DROP TABLE IF EXISTS t3")
    +    sql("DROP TABLE IF EXISTS t4")
    --- End diff --
    
    the same here.


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

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


[GitHub] spark pull request: [SPARK-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#issuecomment-193609126
  
    @rxin Thanks for the input. Let me try to work on it and see if i encounter any issues


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55292434
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/LogicalPlanToSQLSuite.scala ---
    @@ -445,4 +461,86 @@ class LogicalPlanToSQLSuite extends SQLBuilderTest with SQLTestUtils {
           "f1", "b[0].f1", "f1", "c[foo]", "d[0]"
         )
       }
    +
    +  test("SQL generation for generate") {
    --- End diff --
    
    this is getting pretty long. i'd separate it into multiple logically separate test 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 pull request: [SPARK-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55262199
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/CatalystQl.scala ---
    @@ -999,10 +999,16 @@ https://cwiki.apache.org/confluence/display/Hive/Enhanced+Aggregation%2C+Cube%2C
         }
     
         val attributes = clauses.collect {
    -      case Token(a, Nil) => UnresolvedAttribute(a.toLowerCase)
    +      case Token(a, Nil) => UnresolvedAttribute(cleanIdentifier(a.toLowerCase))
         }
     
    -    Generate(generator, join = true, outer = outer, Some(alias.toLowerCase), attributes, child)
    +    Generate(
    +      generator,
    --- End diff --
    
    nvm, you are 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 pull request: [SPARK-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55261341
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/SQLBuilder.scala ---
    @@ -297,6 +322,31 @@ class SQLBuilder(logicalPlan: LogicalPlan, sqlContext: SQLContext) extends Loggi
         )
       }
     
    +  /* This function handles the SQL generation when generators are specified in the
    +   * LATERAL VIEW clause. SQL generation of generators specified in projection lists
    +   * are handled in projectToSQL.
    --- End diff --
    
    are -> is


---
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-12719][SQL] SQL generation support for ...

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

    https://github.com/apache/spark/pull/11563#discussion_r55275359
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/LogicalPlanToSQLSuite.scala ---
    @@ -52,6 +68,8 @@ class LogicalPlanToSQLSuite extends SQLBuilderTest with SQLTestUtils {
         sql("DROP TABLE IF EXISTS parquet_t1")
         sql("DROP TABLE IF EXISTS parquet_t2")
         sql("DROP TABLE IF EXISTS t0")
    +    sql("DROP TABLE IF EXISTS t3")
    +    sql("DROP TABLE IF EXISTS t4")
    --- End diff --
    
    Thanks.. fixed 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