[GitHub] spark pull request #18079: [SPARK-20841][SQL] Support table column aliases i...

2017-05-28 Thread asfgit
Github user asfgit closed the pull request at:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request #18079: [SPARK-20841][SQL] Support table column aliases i...

2017-05-28 Thread maropu
Github user maropu commented on a diff in the pull request:

https://github.com/apache/spark/pull/18079#discussion_r118844314
  
--- Diff: sql/core/src/test/resources/sql-tests/inputs/aliases.sql ---
@@ -0,0 +1,17 @@
+-- Test data.
--- End diff --

ok


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request #18079: [SPARK-20841][SQL] Support table column aliases i...

2017-05-28 Thread hvanhovell
Github user hvanhovell commented on a diff in the pull request:

https://github.com/apache/spark/pull/18079#discussion_r118842688
  
--- Diff: sql/core/src/test/resources/sql-tests/inputs/aliases.sql ---
@@ -0,0 +1,17 @@
+-- Test data.
--- End diff --

How about we name this file `table-aliases.sql`; that seems a little bit 
less confusing.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request #18079: [SPARK-20841][SQL] Support table column aliases i...

2017-05-28 Thread hvanhovell
Github user hvanhovell commented on a diff in the pull request:

https://github.com/apache/spark/pull/18079#discussion_r118842345
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
 ---
@@ -676,9 +676,12 @@ class AstBuilder(conf: SQLConf) extends 
SqlBaseBaseVisitor[AnyRef] with Logging
* Create an aliased table reference. This is typically used in FROM 
clauses.
*/
   override def visitTableName(ctx: TableNameContext): LogicalPlan = 
withOrigin(ctx) {
-val table = 
UnresolvedRelation(visitTableIdentifier(ctx.tableIdentifier))
-
-val tableWithAlias = Option(ctx.strictIdentifier).map(_.getText) match 
{
+val tableId = visitTableIdentifier(ctx.tableIdentifier)
+val table = Option(ctx.tableAlias.identifierList) match {
--- End diff --

...or something like this:
```scala
val outputNames = 
Option(ctx.tableAlias.identifierList).map(visitIdentifierList).getOrElse(Nil)
val table = UnresolvedRelation(visitTableIdentifier(ctx.tableIdentifier), 
outputNames)
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request #18079: [SPARK-20841][SQL] Support table column aliases i...

2017-05-28 Thread hvanhovell
Github user hvanhovell commented on a diff in the pull request:

https://github.com/apache/spark/pull/18079#discussion_r118842292
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
 ---
@@ -676,9 +676,12 @@ class AstBuilder(conf: SQLConf) extends 
SqlBaseBaseVisitor[AnyRef] with Logging
* Create an aliased table reference. This is typically used in FROM 
clauses.
*/
   override def visitTableName(ctx: TableNameContext): LogicalPlan = 
withOrigin(ctx) {
-val table = 
UnresolvedRelation(visitTableIdentifier(ctx.tableIdentifier))
-
-val tableWithAlias = Option(ctx.strictIdentifier).map(_.getText) match 
{
+val tableId = visitTableIdentifier(ctx.tableIdentifier)
+val table = Option(ctx.tableAlias.identifierList) match {
--- End diff --

Just if/else? Seems a bit heavy weight to wrap in an option...


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request #18079: [SPARK-20841][SQL] Support table column aliases i...

2017-05-28 Thread hvanhovell
Github user hvanhovell commented on a diff in the pull request:

https://github.com/apache/spark/pull/18079#discussion_r118842186
  
--- Diff: 
sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -711,7 +711,7 @@ nonReserved
 | ADD
 | OVER | PARTITION | RANGE | ROWS | PRECEDING | FOLLOWING | CURRENT | 
ROW | LAST | FIRST | AFTER
 | MAP | ARRAY | STRUCT
-| LATERAL | WINDOW | REDUCE | TRANSFORM | USING | SERDE | 
SERDEPROPERTIES | RECORDREADER
--- End diff --

I have added as much to the non-reserved keyword list as possible (without 
creating ambiguities). The reason for this is that many datasources (for 
instance twitter4j) unfortunately use reserved keywords for column names, and 
working with these was quite cumbersome. I took the pragmatic approach.

If we want to change this, then we need to do the same Hive did and create 
a config flag. We remove them for Spark 3.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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request #18079: [SPARK-20841][SQL] Support table column aliases i...

2017-05-28 Thread maropu
Github user maropu commented on a diff in the pull request:

https://github.com/apache/spark/pull/18079#discussion_r118836821
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala
 ---
@@ -71,6 +84,11 @@ case class UnresolvedInlineTable(
  *   // Assign alias names
  *   select t.a from range(10) t(a);
  * }}}
+ * 
+ * @param functionName name of this table-value function
+ * @param functionArgs list of function arguments
+ * @param outputNames alias names of function output columns. If these 
names given, an analyzer
+ *adds [[Project]] to rename the output columns.
--- End diff --

This fix is not related to this pr though, I modified along with this fix: 
https://github.com/apache/spark/pull/18079/files#diff-b4f9cbed8a042aeb12aeceb13b39d25aR50


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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



[GitHub] spark pull request #18079: [SPARK-20841][SQL] Support table column aliases i...

2017-05-28 Thread maropu
Github user maropu commented on a diff in the pull request:

https://github.com/apache/spark/pull/18079#discussion_r118836800
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveTableValuedFunctions.scala
 ---
@@ -131,8 +131,9 @@ object ResolveTableValuedFunctions extends 
Rule[LogicalPlan] {
 val outputAttrs = resolvedFunc.output
 // Checks if the number of the aliases is equal to expected one
 if (u.outputNames.size != outputAttrs.size) {
-  u.failAnalysis(s"expected ${outputAttrs.size} columns but " +
-s"found ${u.outputNames.size} columns")
+  u.failAnalysis(s"Number of given aliases does not match number 
of output columns. " +
+s"Function name: ${u.functionName}; number of aliases: " +
+s"${u.outputNames.size}; number of output columns: 
${outputAttrs.size}.")
 }
--- End diff --

This fix is not related to this pr though, I modified along with this 
comment: 
https://github.com/apache/spark/pull/18079/files#diff-57b3d87be744b7d79a9beacf8e5e5eb2R604


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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