[GitHub] spark pull request #16679: [SPARK-19272][SQL] Remove the param `viewOriginal...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 #16679: [SPARK-19272][SQL] Remove the param `viewOriginal...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16679#discussion_r97467363
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala 
---
@@ -426,7 +426,9 @@ private[hive] class HiveClientImpl(
 // in the function toHiveTable.
 properties = properties.filter(kv => kv._1 != "comment" && kv._1 
!= "EXTERNAL"),
 comment = properties.get("comment"),
-viewOriginalText = Option(h.getViewOriginalText),
+// In older versions of Spark(before 2.2.0), we expand the view 
original text and store
+// that into `viewExpandedText`, and that should be used in view 
resolution. So we get
+// `viewExpandedText` instead of `viewOriginalText` for viewText 
here.
--- End diff --

IIRC, we do have such a test


---
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 #16679: [SPARK-19272][SQL] Remove the param `viewOriginal...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/16679#discussion_r97467233
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala 
---
@@ -426,7 +426,9 @@ private[hive] class HiveClientImpl(
 // in the function toHiveTable.
 properties = properties.filter(kv => kv._1 != "comment" && kv._1 
!= "EXTERNAL"),
 comment = properties.get("comment"),
-viewOriginalText = Option(h.getViewOriginalText),
+// In older versions of Spark(before 2.2.0), we expand the view 
original text and store
+// that into `viewExpandedText`, and that should be used in view 
resolution. So we get
+// `viewExpandedText` instead of `viewOriginalText` for viewText 
here.
--- End diff --

BTW, do we have a test case to cover the backward compatibility? That 
means, Spark 2.2.0 still can use/call the view created by Spark 2.1.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 #16679: [SPARK-19272][SQL] Remove the param `viewOriginal...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/16679#discussion_r97390464
  
--- Diff: 
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala 
---
@@ -426,7 +426,6 @@ private[hive] class HiveClientImpl(
 // in the function toHiveTable.
 properties = properties.filter(kv => kv._1 != "comment" && kv._1 
!= "EXTERNAL"),
 comment = properties.get("comment"),
-viewOriginalText = Option(h.getViewOriginalText),
 viewText = Option(h.getViewExpandedText),
--- End diff --

Could you add a comment in this line to explain why we are using 
`h.getViewExpandedText` instead of `h.getViewOriginalText`?


---
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 #16679: [SPARK-19272][SQL] Remove the param `viewOriginal...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16679#discussion_r97295841
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
@@ -528,8 +528,10 @@ case class DescribeTableCommand(
   private def describeViewInfo(metadata: CatalogTable, buffer: 
ArrayBuffer[Row]): Unit = {
 append(buffer, "", "", "")
 append(buffer, "# View Information", "", "")
-append(buffer, "View Original Text:", 
metadata.viewOriginalText.getOrElse(""), "")
 append(buffer, "View Expanded Text:", metadata.viewText.getOrElse(""), 
"")
--- End diff --

nit: not `Expanded`


---
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 #16679: [SPARK-19272][SQL] Remove the param `viewOriginal...

[jiangxb1987]

GitHub user jiangxb1987 opened a pull request:

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

[SPARK-19272][SQL] Remove the param `viewOriginalText` from `CatalogTable`

## What changes were proposed in this pull request?

Hive will expand the view text, so it needs 2 fields: originalText and 
viewText. Since we don't expand the view text, but only add table properties, 
perhaps only a single field `viewText` is enough in CatalogTable.

This PR brought in the following changes:
1. Remove the param `viewOriginalText` from `CatalogTable`;
2. Update the output of command `DescribeTableCommand`.

## How was this patch tested?

Tested by exsiting test cases, also updated the failed test cases.

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

$ git pull https://github.com/jiangxb1987/spark catalogTable

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

https://github.com/apache/spark/pull/16679.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 #16679


commit 7be047715dd82b56ca70ef9292a75e4457849747
Author: jiangxingbo 
Date:   2017-01-23T09:10:59Z

Remove viewOriginalText from CatalogTable.




---
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